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

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

[Ben Collins]

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

On 64-bit QorIQ platforms like T4240, the CPU supports 40-bit addressing
and it's safe to move resources to the upper bounds of the 1TiB limit to
make room for > 64GiB of memory. The PAMU driver does not account for
this, however.

Setup fsl,pamu driver to make use of the full 40-bit addressing space
when configuring liodn's that may have been configured in this range.
Specifically the e5500 and e6500 CPUs.

Signed-off-by: Ben Collins <bcollins@kernel.org>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: iommu@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
---
 drivers/iommu/fsl_pamu.c        | 5 +++--
 drivers/iommu/fsl_pamu.h        | 7 +++++++
 drivers/iommu/fsl_pamu_domain.c | 5 +++--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index f37d3b0441318..ceb352f824010 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -198,7 +198,7 @@ int pamu_config_ppaace(int liodn, u32 omi, u32 stashid, int prot)
 
 	/* window size is 2^(WSE+1) bytes */
 	set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE,
-	       map_addrspace_size_to_wse(1ULL << 36));
+	       map_addrspace_size_to_wse(1ULL << PAMU_MAX_PHYS_BITS));
 
 	pamu_init_ppaace(ppaace);
 
@@ -475,7 +475,8 @@ static void setup_liodns(void)
 			ppaace = pamu_get_ppaace(liodn);
 			pamu_init_ppaace(ppaace);
 			/* window size is 2^(WSE+1) bytes */
-			set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35);
+			set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE,
+			       (PAMU_MAX_PHYS_BITS - 1));
 			ppaace->wbah = 0;
 			set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0);
 			set_bf(ppaace->impl_attr, PAACE_IA_ATM,
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index 36df7975ff64d..5d88871610cfd 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -42,6 +42,13 @@ struct pamu_mmap_regs {
 	u32 olal;
 };
 
+/* Physical addressing capability */
+#if defined(CONFIG_E6500_CPU) || defined(CONFIG_E5500_CPU)
+#define PAMU_MAX_PHYS_BITS	40
+#else
+#define PAMU_MAX_PHYS_BITS	36
+#endif
+
 /* PAMU Error Registers */
 #define PAMU_POES1 0x0040
 #define PAMU_POES2 0x0044
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;
 
 	return &dma_domain->iommu_domain;
-- 
2.49.0


-- 
 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 --]

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

[Jason Gunthorpe]

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..

Jason

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

[Ben Collins]

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

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:

Author: Jason Gunthorpe <jgg@ziepe.ca>
Date:   Wed Sep 13 10:43:38 2023 -0300

    iommu/fsl_pamu: Implement a PLATFORM domain

       /*
        * FIXME: This isn't creating an unmanaged domain since the
        * default_domain_ops do not have any map/unmap function it doesn't meet
        * the requirements for __IOMMU_DOMAIN_PAGING. The only purpose seems to
        * allow drivers/soc/fsl/qbman/qman_portal.c to do
        * fsl_pamu_configure_l1_stash()
        */

The logic hasn't really been touched in 10 years.

-- 
 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 --]

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

[Jason Gunthorpe]

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 :)

Jason

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

[Ben Collins]

[-- 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 --]