Message-ID: <fhb4woejzh3r6v5dxvdiopnsbuwstucfuuzbiymxg4wrxrjc7t@dt3z3utq6lwd>
Date: Thu, 9 Oct 2025 21:25:01 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Robin Murphy <robin.murphy@....com>
Cc: Charan Teja Kalla <charan.kalla@....qualcomm.com>, joro@...tes.org,
        will@...nel.org, saravanak@...gle.com, conor+dt@...nel.org,
        robh@...nel.org, mchehab@...nel.org, bod@...nel.org,
        krzk+dt@...nel.org, abhinav.kumar@...ux.dev,
        vikash.garodia@....qualcomm.com, dikshita.agarwal@....qualcomm.com,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        bjorn.andersson@....qualcomm.com, linux-media@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, iommu@...ts.linux.dev
Subject: Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices

On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
> > On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
> > > On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
> > > > 
> > > > On 9/29/2025 3:50 PM, Robin Murphy wrote:
> > > > > > USECASE [1]:
> > > > > > -----------
> > > > > > Video IP, 32bit, have 2 hardware sub blocks(or can be called as
> > > > > > functions) called as pixel and nonpixel blocks, that does decode and
> > > > > > encode of the video stream. These sub blocks are __configured__ to
> > > > > > generate different stream IDs.
> > > > > 
> > > > > So please clarify why you can't:
> > > > > 
> > > > > a) Describe the sub-blocks as individual child nodes each with their own
> > > > > distinct "iommus" property
> > > > > 
> > > > 
> > > > Thanks Robin for your time. Sorry for late reply as I really didn't have
> > > > concrete answer for this question.
> > > > 
> > > > First let me clarify the word "sub blocks" -- This is just the logical
> > > > separation with no separate address space to really able to define them
> > > > as sub devices. Think of it like a single video IP with 2 dma
> > > > engines(used for pixel and non-pixel purpose).
> > > > 
> > > > I should agree that the child-nodes in the device tree is the easy one
> > > > and infact, it is how being used in downstream.
> > > > 
> > > > For upstream -- Since there is no real address space to interact with
> > > > these sub-blocks(or logical blocks), does it really qualify to define as
> > > > child nodes in the device tree? I see there is some push back[1].
> > > 
> > > Who says you need an address space? Child nodes without "reg" properties,
> > > referenced by name, compatible or phandle, exist all over the place for all
> > > manner of reasons. If there are distinct logical functions with their own
> > > distinct hardware properties, then I would say having child nodes to
> > > describe and associate those properties with their respective functions is
> > > entirely natural and appropriate. The first example that comes to mind of
> > > where this is a well-established practice is PMICs - to pick one at random:
> > > Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> > 
> > Logical function, that's correct. And also note, for PMICs that practice
> > has bitten us back. For PM8008 we switched back to a non-subdevice
> > representation.
> > 
> > > For bonus irony, you can't take the other approaches without inherently
> > > *introducing* a notional address space in the form of your logical function
> > > IDs anyway.
> > > 
> > > >     > or:
> > > > > 
> > > > > b) Use standard "iommu-map" which already supports mapping a masked
> > > > > input ID to an arbitrary IOMMU specifier
> > > > > 
> > > > 
> > > > I think clients is also required to program non-zero smr mask, where as
> > > > iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
> > > > am unable to catch your thought here.
> > > An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
> > > is. The fact that Linux's parsing code only works properly for #iommu-cells
> > > = 1 is not really a DT binding problem (other than it stemming from a loose
> > > assumption stated in the PCI binding's use of the property).
> > 
> > I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> > has only one cell, which is correct even for our platforms. The fact
> > that we need to identify different IOMMU SIDs (and handle them in a
> > differnt ways) is internal to the video device (and several other
> > devices). There is nothing to be handled on the ARM SMMU side.
> 
> Huh? So if you prefer not to change anything, are you suggesting this series
> doesn't need to exist at all? Now I'm thoroughly confused...

Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
#iommu-cells is the best idea.

> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
> because that is the SMMU binding for using SMR masks. It would definitely

I'm sorry. Yes, we have #iommu-cells = <2>.

> not be OK to have some magic property trying to smuggle
> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
> for iommu-map, I don't see what would be objectionable about improving the
> parsing to respect a real #iommu-cells value rather than hard-coding an
> assumption. Yes, we'd probably need to forbid entries with length > 1
> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear

This will break e.g. PCIe on Qualcomm platforms:

                        iommu-map = <0x0   &apps_smmu 0x1400 0x1>,
                                    <0x100 &apps_smmu 0x1401 0x1>;


But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
<2>. It depends on ARM SMMU ignoring the second cell when it's not
present.

> relationship between the input ID and the output specifier falls apart when
> the specifier is complex, but that seems simple enough to implement and
> document (even if it's too fiddly to describe in the schema itself), and
> still certainly no worse than having another property that *is* just
> iommu-map with implicit length = 1.
> 
> And if you want individual StreamIDs for logical functions to be attachable
> to distinct contexts then those functions absolutely must be visible to the
> IOMMU layer and the SMMU driver as independent devices with their own unique
> properties, which means either they come that way from the DT as of_platform
> devices in the first place, or you implement a full bus_type abstraction

Not necessarily. Tegra display driver creates a device for each context
on its own. In fact, using OF to create context devices is _less_
robust, because now the driver needs to sync, checking that there is a
subdevice, that it has probed, etc. Using manually created devices seems
better from my POV.

> which will have to be hooked up to the IOMMU layer. You cannot make IOMMU
> configuration "internal" to the actual client driver which is only allowed
> to bind *after* said IOMMU configuration has already been made.

I'm not sure I follow this, I'm sorry.


-- 
With best wishes
Dmitry

Powered by blists - more mailing lists