Re: [PATCH v6 05/25] iommu/arm-smmu-v3: Move IDR parsing to common functions

Jason Gunthorpe
2026-05-06T09:56:44Z

On Tue, May 05, 2026 at 04:48:08PM +0000, Mostafa Saleh wrote:
> On Tue, May 05, 2026 at 01:27:45PM -0300, Jason Gunthorpe wrote:
> > On Mon, May 04, 2026 at 12:16:13PM +0000, Mostafa Saleh wrote:
> > > On Fri, May 01, 2026 at 09:47:16AM -0300, Jason Gunthorpe wrote:
> > > > On Fri, May 01, 2026 at 11:19:07AM +0000, Mostafa Saleh wrote:
> > > > > Move parsing of IDRs to functions so that it can be re-used
> > > > > +unsigned long smmu_idr5_to_pgsize(u32 reg)
> > > > > +{
> > > > > + unsigned long pgsize_bitmap = 0;
> > > > > +
> > > > > + if (reg & IDR5_GRAN64K)
> > > > > + pgsize_bitmap |= SZ_64K | SZ_512M;
> > > > > + if (reg & IDR5_GRAN16K)
> > > > > + pgsize_bitmap |= SZ_16K | SZ_32M;
> > > > > + if (reg & IDR5_GRAN4K)
> > > > > + pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
> > > > > + return pgsize_bitmap;
> > > > > +}
> > > >
> > > > I think this should include:
> > > >
> > > > > + smmu->oas = smmu_idr5_to_oas(reg);
> > > > > + if (smmu->oas == 52)
> > > > > smmu->pgsize_bitmap |= 1ULL << 42; /* 4TB */
> > > > > - break;
> > > >
> > > > ie it should return the supported page sizes by inspecting all the
> > > > idrs and don't leave this tricky bit to be open coded..
> > >
> > > This way was easier as each function only returns one thing, otherwise
> > > we have to pass stuff by address as we can’t pass the smmu struct as it
> > > is not shared between the drivers.
> > > But no strong opinion, I can change that.
> >
> > Could you share the struct? With multi-compilation you do get two
> > structs with the same name and different layout, it is OK?
> >
> > That would give a cleaner setup too because you can just shove all
> > this into a function and have it setup the same struct which is more a
> > copy and paste than break up into little functions?
>
> Yes, that’s possible, but I didn't want to contaminate the main struct,
> also we can’t compile the main one for KVM because of things such as
> “struct device, struct mutex...”
> May we can do:
> - struct arm_smmu_device_common: Basic stuff (ioaddress, features,
> oas...)
> - arm_smmu_device: For the main driver which inherits arm_smmu_device_common
> - arm_smmu_nested_device for KVM which also inherits arm_smmu_device_common

Less keen on this, it would need more usages to justify it

> What do you think?

I was thinking more like

#define arm_smmu arm_pkvm_smmu

Before the pkvm compile of the shared code.

How does pkvm compilation work anyhow? Is it all rolled into a unique
link, or do you have to worry about symbol conflicts with the main
kernel?

Jason