Hi Both,

Thanks for review and for checking other implementations for this
discrepancy.

On Mon, Mar 03, 2025 at 08:49:02AM +0000, Shameerali Kolothum Thodi wrote:

quoted

-----Original Message-----
From: Jason Gunthorpe <jgg@ziepe.ca>
Sent: Friday, February 28, 2025 7:32 PM
To: Mikołaj Lenczewski <redacted>; Shameerali Kolothum
Thodi [off-list ref]
Cc: ryan.roberts@arm.com; suzuki.poulose@arm.com;
yang@os.amperecomputing.com; catalin.marinas@arm.com;
will@kernel.org; joro@8bytes.org; jean-philippe@linaro.org;
mark.rutland@arm.com; joey.gouly@arm.com; oliver.upton@linux.dev;
james.morse@arm.com; broonie@kernel.org; maz@kernel.org;
david@redhat.com; akpm@linux-foundation.org; nicolinc@nvidia.com;
mshavit@google.com; jsnitsel@redhat.com; smostafa@google.com; linux-
arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
iommu@lists.linux.dev
Subject: Re: [PATCH v2 4/4] iommu/arm: Add BBM Level 2 smmu feature

On Fri, Feb 28, 2025 at 06:24:04PM +0000, Mikołaj Lenczewski wrote:

quoted

For supporting BBM Level 2 for userspace mappings, we want to ensure
that the smmu also supports its own version of BBM Level 2. Luckily, the
smmu spec (IHI 0070G 3.21.1.3) is stricter than the aarch64 spec (DDI
0487K.a D8.16.2), so already guarantees that no aborts are raised when
BBM level 2 is claimed.

Add the feature and testing for it under arm_smmu_sva_supported().

Signed-off-by: Mikołaj Lenczewski <redacted>
---
 arch/arm64/kernel/cpufeature.c                  | 7 +++----
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c     | 3 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h     | 4 ++++
 4 files changed, 13 insertions(+), 4 deletions(-)

This patch looks good, for what it does. However for bisection safety
it should be earlier, before the patches that change the page table
algorithms to be unsafe for the SMMU.

Right, I should have noticed this earlier. Will reorder the patch.

quoted

However, I've heard people talking about shipping chips that have CPUs
with BBML2 but SMMUs without.

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? Otherwise the smmu might experience page faults
when it touches pages from the core that use BBML2, if it does not
support BBML2 itself? Again, I could very well be wrong, will double
check with the reference manuals.

quoted

Though I see your MIDR_REV list is limited, so perhaps that worry
doesn't effect any real chips made with those families? I am trying to
check some NVIDIA products against this list..

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.

We do have implementations that support CPUs with BBLM2 with TLB
conflict aborts and SMMUv3 with BBML2.  So don't think those platforms
be affected by this.  Will check with our hardware folks if there is
anything that will be affected by this.

Also we  have plans to try to use SMMUv3 BBML2 during VM live migration
to split block pages to 4K. I guess, in that case we can enable SMMU BBML2
independent of CPU side.

Thanks,
Shameer

On independently enabling BBML2 on the smmu but not the CPU, this should
be possible. My check was intended to catch the case where the CPU is on
the MIDR allowlist, but the smmu does not support the feature. Whilst
this might still be bad logic, if it is correct, then as long as the CPU
is not in the allowlist sva should not be affected. And bbml2 itself on
the smmu should be safe regardless, as it is stricted than the
corresponding cpu-side feature.

-- 
Kind regards,
Mikołaj Lenczewski