Re: [RFC PATCH 5/5] iommu/arm-smmu-v3: Invoke pm_runtime before hw access

[Pranjal Shrivastava <praan@google.com>]

On Thu, Mar 20, 2025 at 01:22:07PM +0000, Robin Murphy wrote:
> On 20/03/2025 12:54 pm, Jason Gunthorpe wrote:
> > On Thu, Mar 20, 2025 at 07:25:20AM +0000, Pranjal Shrivastava wrote:
> > 
> > > I agree, that we can elide TLBIs if the smmu is asleep (powered-off),
> > > the main reason to wake up the hw is because `arm_smmu_write_ste` issues
> > > a prefetch CMD. Again, it is possible that the smmu sleeps after this op
> > > and the prefetch has no real benefit.
> > 
> > Does any HW caching survive the power off? I was assuming no.
> > 
> > So it is pointless to issue a prefetch and then immediately power off
> > the SMMU and throw away the STE cache.
> > 
> > IOW if you don't know that the SMMU will stay powered up after the
> > attach there is no reason to do a prefetch.
> 
> And even if the caches don't lose state in hardware, we'll explicitly clear
> them on resume anyway. Furthermore I'm not aware of any current SMMU
> implementations where the prefetch command does anything meaningful anyway,
> so right now this is absolutely a non-issue.
> 

Ack. I agree, for the next version, I'll use rpm_get_if_active for
any TLBIs / CFGIs and Prefetches and otherwise elide it.

> > > Anyway, having *any* rpm implementation that causes us to lose TLB
> > > content, will cause a hit on the performance, so maybe this
> > > micro-optimization isn't worth at all?
> > 
> > Right, it was my assumption that you loose the caches. I would guess
> > keeping all the cache SRAM powered is a big part of the power budget...
> > 

Right. I think it's fair to assume that SRAMs aren't retained, even if
they are, we anyway clean everything on resume.

> > > Also, while we're at it, should we have a feature switch for users who'd
> > > like to disable runtime PM for performance reasons? I don't think that
> > > relying on the `power-domain` being present alone is sufficient.
> > 
> > I'm worried about how much this will hurt server workloads. Optimizing
> > invalidation is a worry and you've gone and put in some more atomic
> > write traffic on shared cache lines. Not great :\
> 
> Indeed, that was the reason for wrapping the RPM calls in explicit
> pm_runtime_enabled() checks in SMMUv2 - see the comment in
> arm_smmu_device_probe() - on the assumption that servers wouldn't advertise
> a power domain for the SMMU (not least because we've no support for such a
> thing under ACPI anyway).
> 

Yes, SMMUv2 is what I referred to for this. So..just checking for
pm_runtime_enabled based on dev->power-domain should suffice then?

> Thanks,
> Robin.