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

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:

quoted

quoted

quoted

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.

quoted

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.

quoted

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