Re: [PATCH 1/7] iommu/arm-smmu-v3: Split struct arm_smmu_strtab_cfg.strtab

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Jun 04, 2024 at 01:32:20AM -0700, Nicolin Chen wrote:
> On Mon, Jun 03, 2024 at 07:31:27PM -0300, Jason Gunthorpe wrote:
> 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > index 1242a086c9f948..4769780259affc 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > @@ -612,7 +610,10 @@ struct arm_smmu_s2_cfg {
> >  };
> >  
> >  struct arm_smmu_strtab_cfg {
> > -	__le64				*strtab;
> > +	union {
> > +		struct arm_smmu_ste *linear;
> > +		__le64 *l1_desc;
> > +	} strtab;
> >  	dma_addr_t			strtab_dma;
> >  	struct arm_smmu_strtab_l1_desc	*l1_desc;
> >  	unsigned int			num_l1_ents;
> 
> It looks like we have two "l1_desc" ptrs now in the same struct:
> 	strtab.l1_desc	// raw level-1 descriptor memory
> 	l1_desc		// SW array to store level-2 descriptor memory
> 
> And it gets a bit more confusing that they even use the same error
> prints in arm_smmu_init_strtab_2lvl()...

Yeah, I noticed that too, but failed to come with better names.. The
CD has the same issue

strtab.l1_desc is a pointer to the data structure that the HW fetches
that is the first level of a 2 level strtab, it stores an encoded
dma_addr_t.

cfg.l1_desc is an array of CPU information for each HW L1 entry,
eventually just being the CPU pointer to the L2 STE table.

So they are both the l1 array, just one is a CPU pointer and one is a
HW/DMA pointer.

Let's call strtab.l1_desc --> strtab.l1_table ?

> The "struct arm_smmu_strtab_l1_desc" seems to be only used at one
> place in arm_smmu_init_l2_strtab(). So, how about:

I didn't do it but, it would make some of the maths more obvious 
if we encoded the table structure in the types:

struct arm_smmu_strtab_l2_stes {
	struct arm_smmu_ste l2[256];
};

Jason