linux-kernel - Re: [PATCH 1/2] arm64: dts: qcom: agatti: Fix IOMMU DT properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ifhznj7eadfudzagfrroyfepkti7kkshwkqiboeqgcdpb6g342@2opxunddxcvn>
Date: Sat, 3 Jan 2026 09:43:57 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Sumit Garg <sumit.garg@...nel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, konradybcio@...nel.org, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	akhilpo@....qualcomm.com, vikash.garodia@....qualcomm.com, 
	dikshita.agarwal@....qualcomm.com, robin.clark@....qualcomm.com, lumag@...nel.org, 
	linux-kernel@...r.kernel.org, Sumit Garg <sumit.garg@....qualcomm.com>, 
	Prakash Gupta <guptap@....qualcomm.com>
Subject: Re: [PATCH 1/2] arm64: dts: qcom: agatti: Fix IOMMU DT properties

On Tue, Dec 30, 2025 at 01:24:23PM +0530, Sumit Garg wrote:
> On Tue, Dec 30, 2025 at 09:05:28AM +0200, Dmitry Baryshkov wrote:
> > On Tue, Dec 30, 2025 at 10:45:38AM +0530, Sumit Garg wrote:
> > > On Mon, Dec 29, 2025 at 10:00:02PM +0200, Dmitry Baryshkov wrote:
> > > > On Mon, Dec 29, 2025 at 12:42:57PM +0530, Sumit Garg wrote:
> > > > > From: Sumit Garg <sumit.garg@....qualcomm.com>
> > > > > 
> > > > > Fix IOMMU DT propeties for GPU, display and video peripherals via
> > > > > dropping SMMU stream IDs which relates to secure context bank.
> > > > > 
> > > > > This problem only surfaced when the Gunyah based firmware stack is
> > > > > ported on Agatti replacing the legacy QHEE based firmware stack. Assigning
> > > > > Linux kernel (HLOS) VMID to secure context bank stream IDs is treated
> > > > > as a fault by Gunyah hypervisor which were previously ignored by QHEE
> > > > > hypervisor.
> > > > 
> > > > We are porting the underlaying layer. Can we make it to behave in a way
> > > > similar to the _existing_ software? If DT was a part of the firmware, it
> > > > would have been fine to perform such updates. But as it is not, we
> > > > really should try not to break the interface boundary between firmware
> > > > and the kernel.
> > > 
> > > I support your viewpoint. But in this current case Linux kernel VMID mapping
> > > for secure context bank stream IDs is incorrect. As Konrad mentioned in the
> > > other thread that for secure media streaming use-cases, it rather requires
> > > proper representation via iommu-map such that appropriate VMID can be mapped.
> > 
> > Yes, I understand this part. I'm basically suggesting that Gunyah should
> > ignore this SID when programming the actual SMMU (probably like QHEE
> > did).
> 
> TBH, I generally dislike as to why the firmware based hypervisor
> behaviour should be encoded in the DT.

The DeviceTree describes the world to the operating system. If there is
a hypervisor (or secure world, or other configuration) that is affecting
the (hardware or firmware) interfaces that needs to either be
discoverable at runtime, or represented in the description of those
interfaces.

> One can always argue that Gunyah
> is doing the right thing given the purpose of SMMU stream IDs. And if
> Gunyah has to change that behaviour since Agatti DT was based on QHEE
> behaviour then the same behaviour will get replicated on all other Qcom
> SoCs.
> 

Yes, you can see that mess in sdm845, sc7180, sc7280, and the various
*-el2.dtso; all cases where we need to describe the variations of the
hardware and firmware interfaces presented to the operating system.

> So, I would suggest that kernel should follow the expected behaviour
> which is to not program HLOS VMID for stream IDs dedicated for secure
> context bank.
> 

I agree. As discussed elsewhere, we might still need to manage the
secure context banks in some cases, but that needs special handling
anyways.

Moreover, it's two context banks (unsecure and secure), but the current
description defines a single context bank capturing both the unsecure
and secure streams.

Regards,
Bjorn

> > 
> > > 
> > > > 
> > > > I'm fine with the change, but I think we should make sure that Gunyah
> > > > doesn't break existing device trees.
> > > 
> > > Sure, this change isn't breaking any DT ABI but brings compliance to
> > > usage of stream IDs.
> > 
> > Actually, it does, as you can see from Rob's email. You didn't perform
> > make dtbs_check while submitting this patch.
> 
> Okay, I missed that. Looks like the GPU DT node already defined minItems
> for the iommus property but the others didn't. I will add following
> change in the next version to fix the warnings.
> 
> Irrespective on that, the DT forwards and backwards ABI compatibility is
> being maintained with the changes being proposed in this patch-set.
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml
> index f0cdb5422688..e5e62c5bab42 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml
> @@ -33,6 +33,7 @@ properties:
>        - const: core
> 
>    iommus:
> +    minItems: 1
>      maxItems: 2
> 
>    interconnects:
> diff --git a/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
> index 3f3ee82fc878..8d4371697867 100644
> --- a/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
> @@ -42,6 +42,7 @@ properties:
>        - const: vcodec0_bus
> 
>    iommus:
> +    minItems: 2
>      maxItems: 5
> 
>    interconnects:
> 
> -Sumit