[PATCH v5 01/17] iommu/arm-smmu-v3: Make STE programming independent of the callers

[Jason Gunthorpe]

On Tue, Feb 27, 2024 at 12:43:18PM +0000, Will Deacon wrote:
> On Fri, Feb 23, 2024 at 11:18:41AM -0400, Jason Gunthorpe wrote:
> > On Thu, Feb 22, 2024 at 05:43:46PM +0000, Will Deacon wrote:
> > > On Wed, Feb 21, 2024 at 10:08:18AM -0400, Jason Gunthorpe wrote:
> > > > On Wed, Feb 21, 2024 at 01:49:23PM +0000, Will Deacon wrote:
> > > > 
> > > > > Very roughly, yes, although I'd go further and just return a bitmap of
> > > > > used qwords instead of tracking these bits. Basically, we could have some
> > > > > #defines saying which qwords are used by which configs, 
> > > > 
> > > > I don't think this will work well for CD's EPD0 case..
> > > > 
> > > > 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));
> > > > 
> > > > 	/* 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);
> > > > 	}
> > > > }
> > > 
> > > Please can you explain more about the issue here? I know what EPDx are,
> > > but I'm not understanding why they're problematic. This presumably
> > > involves a hitless transition to/from an aborting CD?
> > 
> > When a process using SVA exits uncleanly the MM is released so the
> > SMMU HW must stop chasing the page table pointers since all that
> > memory will be freed.
> > 
> > However, in an unclean exit we can't control the order of shutdown so
> > something like uacce or RDMA may not have quieted the DMA device yet.
> > 
> > So there is a period during shutdown where the mm has been released
> > and the device is doing DMA, the desire is that the DMA continue to be
> > handled as a PRI and the SW will return failure for all PRI requests.
> > 
> > Specifically we do not want to trigger any dmesg log events during
> > this condition.
> 
> Curious, but why is it problematic to log events? As you say, it's an
> "unclean" exit, so it doesn't seem that unreasonable to me.

Well, I would defer to Jean-Philippe, but I can understand the
logic. A user ctrl-c's their application it is not nice to get some
dmesg logs from that.

I recall he felt strongly about this, we had some discussion about it
related to the mmu notifiers back when the iommu drivers were all
updated to the new notifier API I built...

> > Jean-Philippe came up with this solution where we hitlessly use EPD0
> > in release to allow the mm to release the page table while continuing
> > to use the PRI flow.
> >
> > So it is going from a "SVA domain with a page table" to a "SVA domain
> > without a page table but EPD0 set", hitlessly.
> 
> Ok, and so the reason this adds complexity is because the set of used
> bits/qwords changes based on something other than the cfg? 

There is no cfg for CD entries? I think it is the same issue as SHCFG,
qw1 of CD is not neatly split and qw0/1 are both changing for the EPD0
case - we also zero the unused TCR/TTB.

> I think it's a pretty weak argument for field vs qwords, but it's a
> good counter-example to my naive approach of per-config masks, so
> thanks.

We could do EPD0 just by editting in place, it would be easy to code,
but the point of this design was to never edit a descriptor in place.

Jason