Re: [PATCH] fsl_pamu: Use 40-bits for addressing where appropriate

[Ben Collins <ben.collins@linux.dev>]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]

On Tue, Apr 22, 2025 at 08:43:05PM -0500, Jason Gunthorpe wrote:
> On Tue, Apr 22, 2025 at 06:21:32PM -0400, Ben Collins wrote:
> > On Tue, Apr 22, 2025 at 04:09:21PM -0500, Jason Gunthorpe wrote:
> > > On Mon, Apr 21, 2025 at 10:46:19PM -0400, Ben Collins wrote:
> > > > diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> > > > index 30be786bff11e..a4bc6482a00f7 100644
> > > > --- a/drivers/iommu/fsl_pamu_domain.c
> > > > +++ b/drivers/iommu/fsl_pamu_domain.c
> > > > @@ -214,9 +214,10 @@ static struct iommu_domain *fsl_pamu_domain_alloc(unsigned type)
> > > >  	INIT_LIST_HEAD(&dma_domain->devices);
> > > >  	spin_lock_init(&dma_domain->domain_lock);
> > > >  
> > > > -	/* default geometry 64 GB i.e. maximum system address */
> > > > +	/* Set default geometry based on physical address limit. */
> > > >  	dma_domain->iommu_domain. geometry.aperture_start = 0;
> > > > -	dma_domain->iommu_domain.geometry.aperture_end = (1ULL << 36) - 1;
> > > > +	dma_domain->iommu_domain.geometry.aperture_end =
> > > > +		(1ULL << PAMU_MAX_PHYS_BITS) - 1;
> > > >  	dma_domain->iommu_domain.geometry.force_aperture = true;
> > > 
> > > What on earth does this even do? There is no map_range() callback in
> > > this driver, so nothing should be reading geometry..
> > 
> > I dunno, but your "FIXME this is broken" comments are all over it from a
> > year and a half ago:
> 
> Yes, I know, but you are changing this - are you changing it because
> something is broken without making this change, if so what, or are you
> changing it because it looked like it needed changing?
> 
> > The logic hasn't really been touched in 10 years.
> 
> Yeah, so I'm surprised someone still cares about it :)

Ironically, this patch sat collecting dust for 10 years until recently
when I revived my T4240 system :)

The change is mostly to be "correct" in as much as the code can be
correct when it's a little broken. Does it fix anything? It does. PAMU
gets a little miffed about my liodn tags being up near the 1TiB boundary.

If it makes you feel any better about it, I've added fsl_pamu to the
list of things I'm fixing for this board, for the hell of it. I have a
pex8724 to program to get 2 of the 4 nvme slots up, a DPAA driver to
build to get the RIO up (and likely some tweaking to program the IDT SRIO
switch to work across the fabric).

IOW, I think I can maybe help get rid of your FIXMEs. For reference:

https://www.manualslib.com/manual/1061198/Vvdn-T4mfcs-Scaleout.html

Plus I have a couple P4080 systems for regression testing with a 36-bit
addressing space.

-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]