Re: [PATCH v2 2/8] iommu/arm-smmu-v3: Use S2FWB when available

Jason Gunthorpe
2024-09-03T23:35:15Z

On Tue, Sep 03, 2024 at 07:57:01AM +0000, Mostafa Saleh wrote:

> Basically, I believe we shouldn’t set FWB blindly just because it’s supported,
> I don’t see how it’s useful for stage-2 only domains.

And the only problem we can see is some niche scenario where incoming
memory attributes that are already requesting cachable combine to a
different kind of cachable?

> And I believe making assumptions about VFIO (which actually is not correctly
> enforced at the moment) is fragile.

VFIO requiring cachable is definately not fragile, and it also sets
the IOMMU_CACHE flag to indicate this. Revising VFIO to allow
non-cachable would be a signficant change and would also change what
IOMMU_CACHE flag it sets.

> and we should only set FWB for coherent
> devices in nested setup only where the VMM(or hypervisor) knows better than
> the VM.

I don't want to touch the 'only coherent devices' question. Last time
I tried to do that I got told every option was wrong.

I would be fine to only enable for nesting parent domains. It is
mandatory here and we definitely don't support non-cachable nesting
today. Can we agree on that?

Keep in mind SMMU S2FWB is really new and probably very little HW
supports it right now. So we are not breaking anything existing
here. IMHO it is better to always enable the stricter features going
forward, and then evaluate an in-kernel opt-out if someone comes with
a concrete use case.

Jason