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

Will Deacon
2024-02-27T12:43:39Z

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.

> 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? 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.

> BTW Michael's self test won't be in part 1 because it needs the ops to
> be restored in order to work (now done in part 2), and has a few other
> more minor dependencies on part 2 and 3.

That's a pity, but fair enough.

Will