Re: [PATCH v2 4/4] iommu/arm: Add BBM Level 2 smmu feature

["Mikołaj Lenczewski" <miko.lenczewski@arm.com>]

On Mon, Mar 03, 2025 at 12:52:55PM -0400, Jason Gunthorpe wrote:
> On Mon, Mar 03, 2025 at 10:31:02AM +0000, Mikołaj Lenczewski wrote:
> 
> > > > On such a system it seems like your series would break previously
> > > > working SVA support because this patch will end up disabling it?
> > 
> > Perhaps my understanding is flawed here, but I was under the impression
> > that with SVA both the core and smmu MUST support BBML2 to use it safely
> > for core translations? 
> 
> Yes
> 
> But today's kernel does not use BBML2 in the CPU or the SMMU so it is
> compatible with everything.
> 
> So it is an upgrade issue, going from today's kernel without any BBML2
> support to tomorrow's kernel that does then you loose SVA on
> previously working HW.

I see what you mean.

If you have hardware that supports cpu BBML2 but doesn't support smmu
BBML2, either by virtue of having a pre-SMMUv3.2 smmu, or by only
implementing BBML1, the simplest option I see is then to disable BBML2
globally in the kernel. A bad klude, but would at least leave current
SVA working.

> > Hopefully, as you say, the MIDR list restricts the breakage to a limited
> > (ideally, zero-size) set of implementations which advertise BBML2
> > without conflict aborts, but which do not support BBML2 on the smmu.
> > 
> > However, if my understanding of the BBML2 feature and how it interacts
> > with SVA is flawed, this will obviously be something for me to fix.
> 
> Lets hope, I was not able to discover any NVIDIA platforms that have
> an issue with this series as is.
> 
> But every addition to the MIDR list will require some consideration :\

Yes, I agree. I believe that for larger cores, this is a guarantee that
implementors will be able to make when adding themselves to the list.
They know what smmu their core uses, and that tends to be
performance-focused (hence more likely to use an smmu that support
BBML2).

For smaller cores, i.e. mobile cores, this becomes a larger issue as you
may have a higher likelyhood of pairing such a core with an
older/insufficient smmu version. So, careful consideration is required.

> > On independently enabling BBML2 on the smmu but not the CPU, this should
> > be possible.
> 
> What about the reverse? Could we disable BBML2 on the CPU side on a
> per-mm basis? Ie when an old SMMU attaches with disable the
> incompatible feature? Not something for this series, but if we get
> into trouble down the road

I agree in principle with having, as you point out, some mechanism
to disable BBML2 when an smmu without BBML2 is used. However, I think
that the complexity of such a mechanism depends on how the BBML2 feature
is used by future patches.

For example, if we use BBML2 for kernel mappings, does that require us
to repaint all kernel mappings when disabling BBML2 on smmu attach? I
am not sure, but definitely something to be worked out.

-- 
Kind regards,
Mikołaj Lenczewski