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

[Jason Gunthorpe]

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