Re: [PATCH 04/22] arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris

[Dmitry Baryshkov]

On Mon, Aug 10, 2026 at 10:27:31PM +0530, Vikash Garodia wrote:
> On 8/10/2026 5:40 PM, Dmitry Baryshkov wrote:
> > On Sat, Aug 08, 2026 at 09:18:42PM +0530, Vikash Garodia wrote:
> > > On 8/7/2026 6:48 PM, Bryan O'Donoghue wrote:
> > > > On 07/08/2026 11:22, Vikash Garodia wrote:
> > > > > > > > I don't like the idea of this series, because it_again_
> > > > > > > > doesn't tell us
> > > > > > > > the truth about the hardware. This typicall ends up with
> > > > > > > > bigger problems
> > > > > > > honestly...thats all the info i have about the vpu hardware that it
> > > > > > > restricts non pixel to DMA from the 0-600MB range. The same i have been
> > > > > > > trying for a year now
> > > > > > You are not honest here. You also know that there are secure streams,
> > > > > > which have to use their own IOMMU SIDs. And some of them, as far as I
> > > > > > remember, also have memory range restrictions.
> > > > > please read the commit description again, the answer is there.
> > > > 
> > > > So I don't necessarily get all of the detail out of the commit log myself.
> > > > 
> > > > Could you give some detail to address Dmitry's point.
> > > > 
> > > > The question as I read it is - are all of the other potential SIDs
> > > > covered by this change ?
> > > > 
> > > 
> > > I get the query as "secure streams also have their dedicated reserve
> > > regions, so how does this approach helps" - This patch does not reserve any
> > > IOVA for secure streams, only the forward looking subnode can assign
> > > specific reserve for specific streams.
> > > The patch enforces a common IOVA across all streams, and is good enough to
> > > fix the problem we have w.r.t device reset.
> > 
> > No, it's not good enough. It defines that both non-secure streams use
> > the provided memory range, it doesn't provide a natural way to later
> > _expand_ it to support secure subnodes, etc.
> > 
> > We know that there is a problem. We already have been bitten by not
> > describing the hardware as is and using band-aids. Can we now learn the
> > lesson and write a proper hardware description?
> > 
> > > 
> > > > > > So, if we land these patches, how do extend it later to account for all
> > > > > > of that?
> > > > > > 
> > > > > forward looking design would be subnode, which we can land ontop of this
> > > > > series.
> > > > 
> > > > Yes it should be possible to branch to make subnodes work on-top of this
> > > > - accepting that once this lands it becomes ABI and support for this
> > > > method must be sustained, even after sub-nodes land.
> > > > 
> > > 
> > > Thats the plan. This goes as ABI with subnode to land on top of it.
> > 
> > So, do you actually plan to support both ABIs? This would also mean
> > moving reserved regions to the subnode.
> 
> How is that different from moving the reserve region when stream IDs would
> also need to move, so be it for the associated memory regions.
> 
> > What prevents us from landing subnodes straight away?
> > 
> 
> You are well aware of them, but still asking the same. Reasons,
> 1. We have been attempting the subnodes for almost a year with multiple
> pushbacks from multiple maintainers. We are closer and working on it to post
> for iris, followed by venus, once review is acceptable for iris.

I think that we should have concentrated on it.  From my point of view,
the subnodes solution is ready and the last obstacle was the
under-informing commit message. As you can see, Rob has (had?) questions
about this solution too.

> 2. Current solution, in this series, is much easy to apply for all kernels
> and can land faster so that it can fix the reset issue we have for already
> enabled devices.
> 
> Both from timewise and simplicity wise, this proposal is made to address the
> reset issue, while subnode can land ontop of this, without breaking ABI.

Or rather breaking the ABI in the same way as it is without the fix.

I'll give another reviewer's thought to it tomorrow...

-- 
With best wishes
Dmitry