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

[Mostafa Saleh <smostafa@google.com>]

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


What do you think?

Thanks,
Mostafa
> 
> Jason