Re: [PATCH v5 03/17] iommu/arm-smmu-v3: Move arm_smmu_rmr_install_bypass_ste()

Jason Gunthorpe
2024-02-13T17:30:49Z

On Tue, Feb 13, 2024 at 03:37:43PM +0000, Mostafa Saleh wrote:
> Hi Jason,
>
> On Tue, Feb 06, 2024 at 11:12:40AM -0400, Jason Gunthorpe wrote:
> > Logically arm_smmu_init_strtab() is the function that allocates and
> > populates the stream table with the initial value of the STEs. After this
> > function returns the stream table should be fully ready.
> >
> > arm_smmu_rmr_install_bypass_ste() adjusts the initial stream table to force
> > any SIDs that the FW says have IOMMU_RESV_DIRECT to use bypass. This
> > ensures there is no disruption to the identity mapping during boot.
> >
> > Put arm_smmu_rmr_install_bypass_ste() into arm_smmu_init_strtab(), it
> > already executes immediately after arm_smmu_init_strtab().
> >
> > No functional change intended.
>
> I think arm_smmu_init_strtab is quite low level to abstract FW configuration in it.
> For example in KVM[1] we'd re-use a big part of this driver and rely on similar
> low-level functions. But no strong opinion.

I'm happy to drop this patch, if no strong opinion I will leave it

> [1] https://lore.kernel.org/kvmarm/20230201125328.2186498-1-jean-philippe@linaro.org/

I saw this but I didn't try to figure out too much what it is
doing.. It looks like a new iommu driver that re-uses alot of the
datastructures of SMMU but has a different HW facing API based on
hypercalls?

It seems interesting, but my knee jerk reaction would be that a new
iommu driver proposal needs to implement the new iommu core APIs, not
the old stuff. IMHO, when this progresses past a RFC it needs to come
as a proper submission of just the iommu driver, in the normal way.

I also wonder about the wisdom of sharing so much code. Code sharing
is not unconditionally good if it doesn't have a robust abstraction..

Jason