On Tue, Mar 18, 2025 at 01:36:55PM +0000, Robin Murphy wrote:
> On 12/03/2025 1:30 pm, Connor Abbott wrote:
> > On Wed, Mar 12, 2025 at 8:49 AM Will Deacon <will@xxxxxxxxxx> wrote:
> > > On Tue, Mar 11, 2025 at 04:01:00PM -0400, Connor Abbott wrote:
> > > > On Tue, Mar 11, 2025 at 2:11 PM Will Deacon <will@xxxxxxxxxx> wrote:
> > > > > On Tue, Mar 04, 2025 at 11:56:50AM -0500, Connor Abbott wrote:
> > > > > > + /*
> > > > > > + * If the device is on and we changed the setting, update the register.
> > > > > > + */
> > > > > > + if (stall_changed && pm_runtime_get_if_active(smmu->dev) > 0) {
> > > > > > + spin_lock_irqsave(&smmu_domain->cb_lock, flags);
> > > > > > +
> > > > > > + u32 reg = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_SCTLR);
> > > > > > +
> > > > > > + if (enabled)
> > > > > > + reg |= ARM_SMMU_SCTLR_CFCFG;
> > > > > > + else
> > > > > > + reg &= ~ARM_SMMU_SCTLR_CFCFG;
> > > > > > +
> > > > > > + arm_smmu_cb_write(smmu, cfg->cbndx, ARM_SMMU_CB_SCTLR, reg);
> > > > >
> > > > > Are you sure you don't need TLB invalidation for this to take effect? I
> > > > > think some fields in the SCTLR can be cached in the TLB but you'll need
> > > > > to check whether or not that applies to CFCFG.
> > > > >
> > > >
> > > > I think it should be fine because CFCFG only controls behavior when
> > > > there's a context fault and there can't be TLB entries for entries
> > > > that cause a context fault: "The architecture permits the caching of
> > > > any translation table entry that has been returned from memory without
> > > > a fault and that does not, as a result of that entry, cause a
> > > > Translation Fault or an Access Flag fault."
> > >
> > > Ok, but what about other types of fault? For example, a permission fault
> > > or an address size fault?
> > >
> > I'm not sure, but the pseudocode for context faults mentions
> > resampling CFCFG after a fault happens ("We have a fault and must
> > resample FSR, CFCFG and HUPCF") so I don't think it would be legal to
> > cache it. Also in practice this does seem to work. Does that answer
> > it?
>
> FWIW I checked with the former MMU-500 design lead, and although he doesn't
> remember the exact details he's pretty confident that they wouldn't have
> cached anything fault-related, so at least from our side I'd consider this
> OK.
Thanks for checking, Robin.
Connor -- please can you stick some of the above in a comment prior
to the CFCFG munging so I don't "rediscover" the problem in the future?
Will
