Re: [PATCH v5 05/27] iommu/arm-smmu-v3: Make CD programming use arm_smmu_write_entry()

[Mostafa Saleh <smostafa@google.com>]

On Tue, Mar 26, 2024 at 03:30:55PM -0300, Jason Gunthorpe wrote:
> On Sat, Mar 23, 2024 at 01:02:15PM +0000, Mostafa Saleh wrote:
> > > +static void arm_smmu_get_cd_used(const __le64 *ent, __le64 *used_bits)
> > > +{
> > > +	used_bits[0] = cpu_to_le64(CTXDESC_CD_0_V);
> > > +	if (!(ent[0] & cpu_to_le64(CTXDESC_CD_0_V)))
> > > +		return;
> > > +	memset(used_bits, 0xFF, sizeof(struct arm_smmu_cd));
> > 
> > This is a slightly different approach than what the driver does for STEs,
> > where it explicitly sets the used bits. Is there a reason for that?
> 
> It is just more compact this way

IMHO, it seems too much to have this mechanism for CDs for just one
SVA case, but I'll need to go through the whole seires first to make
sure I am not missing anything.

> > > +	/* EPD0 means T0SZ/TG0/IR0/OR0/SH0/TTB0 are IGNORED */
> > > +	if (ent[0] & cpu_to_le64(CTXDESC_CD_0_TCR_EPD0)) {
> > > +		used_bits[0] &= ~cpu_to_le64(
> > > +			CTXDESC_CD_0_TCR_T0SZ | CTXDESC_CD_0_TCR_TG0 |
> > > +			CTXDESC_CD_0_TCR_IRGN0 | CTXDESC_CD_0_TCR_ORGN0 |
> > > +			CTXDESC_CD_0_TCR_SH0);
> > > +		used_bits[1] &= ~cpu_to_le64(CTXDESC_CD_1_TTB0_MASK);
> > > +	}
> > > +}
> > 
> > We should add a comment about EPD1 maybe?
> 
> Driver doesn't use TTB1?

Yes, it's not immediately obvious why we ignore EPD1, so maybe it's
worth a comment to highlight that, but no strong opinion.

Thanks,
Mostafa