Re: [PATCH v2 0/2] iommu: Allow passing custom allocators to pgtable drivers

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Nov 22, 2023 at 05:23:34PM +0000, Robin Murphy wrote:
> > Gaurav doesn't need a custom allocator, he needs a way to manage
> > sharing page table memory with the hypervisor. A different set of ops
> > that don't replace the entire allocator would be fine for them.
> > 
> > The issue with taking away the alloc_page is that it complicates the
> > ability for the common code to use the struct page fields and more. We
> > already know we are going to need those to do some of the things that
> > are being asked for, eg RCU freeing and refcounting.
> 
> Oh come on, for all the things you claim are simple, you don't think it's
> trivial to make the fancy stuff conditional? (Hint: we already have control
> flow all over which is conditional on the cfg and/or calling context, and
> that future stuff may well end up wanting to be conditional in its own right
> anyway since not all io-pgtable-arm users will need or want it).

I really don't want to make it conditional! I'd want to have one
common set of radix algorithms shared by the enterprise IOMMUs, so we
can actually improve those drivers and get all the special features
people want without writing everything 5 times.

> > Here I am not talking about perfection, I am concerned about messing
> > up work that I now see as important for other iommu drivers related
> > things by making a hack for a DRM driver (which as I've said has,
> > unravelling these hacks has been traumatic for many of us
> > historically)
> 
> In your own words, this is a community effort. You do not own this code, and
> you do not get to prevent the community from solving multiple problems they
> have now, in a way which is reasonable now, just because *you* don't like
> it, and *you* believe it might possibly make it harder to do something at
> some indeterminate point in future. Your objection is noted, but as above, I
> for one do not see a convincing basis for it anyway.

I've said already it is my opinion, if you want to bring another one
then you can particpate too. Thank you for noting my objection.

> You know what *the community* sees as important? Not having to maintain
> multiple copies of near-identical pagetable code, because maintenance is an
> established and very real concern. 

What I heard at LPC is we badly need to stop maintaining independent
radix algorithms across at least the enterprise iommu drivers. This
was clearly an articulated need for many different people *from the
community*.

> This is why we have the io-pgtable
> library in the first place, and why we adapted io-pgtable-arm to support
> panfrost when that came along. If supporting panfrost and panthor as
> io-pgtable users really does ever become a practical problem at some point
> in the future, we will work to address that problem at that point.

You mean Joao or myself will get stuck with it. Not "we". :(

> meantime, we can get on with the common goal of making Linux do the things
> people want it to do, in the best way that the maintainers are happy to
> maintain. This is not "a hack for a DRM driver", it's a simple and
> convenient generalisation of part of io-pgtable, in keeping with the design
> of io-pgtable (user-provided callbacks are nothing new), and
> straightforwardly serving at least two completely different use-cases
> already.

I've already explained to Boris, and he agreed, that it is the *wrong*
generalization because it is implementing a generally useful algorithm
in the DRM driver and keeping it out of the core code.

If we deliberately do the wrong thing then, yes, it is a hack.

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel