Re: [PATCH v2 06/11] iommu/arm-smmu-v3: Introduce arm_smmu_s2_parent_tlb_ invalidation helpers

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Apr 15, 2025 at 09:50:42AM -0300, Jason Gunthorpe wrote:
> struct invalidation_op {
>     struct arm_smmu_device *smmu;
>     enum {ATS,S2_VMDIA_IPA,S2_VMID,S1_ASID} invalidation_op;
>     union {
>         u16 vmid;
>         u32 asid;
>     	u32 ats_id;
>     };
>     refcount_t users;
> };
> 
> Then invalidation would just iterate over this list following each
> instruction. 
> 
> When things are attached the list is mutated:
>  - Normal S1/S2 attach would reuse an ASID for the same instance or
>    allocate a new list entry, users keeps track of ID sharing
>  - VMID attach would use the VMID of the vSMMU
>  - ATS enabled would add entries for each PCI device instead of the
>    seperate ATS list

Interesting. I can see it generalize all the use cases.

Yet are you expecting a big list combining TLBI and ATC_INV cmds?

I think the ATC_INV entries doesn't need a refcount? And finding
an SID (to remove the device for example) would take long, when
there are a lot of entries in the list?

Should the ATS list still be separate, or even an xarray?

> To do this without locking on the invalidation side would require
> using RCU to manage the list, which suggests it is probably an array
> that is re-allocated each time it is changed.
> 
> That means some fancy algorithms to copy and mutate the array, deal
> with error cases and sort it (ATS must follow ID, want things grouped
> by instance).
> 
> There is some tricky memory barriers needed and RCU would require that
> SMMU unplug do a synchronize_rcu(). IIRC riscv did this in their
> driver.

I will refer to their driver. Yet, I wonder what we will gain from
RCU here? Race condition? Would you elaborate with some use case?

> But the end result is we fully disconnect the domain from the smmu
> instance and all domain types can be shared across all instances if
> they support the pagetable layout. The invalidation also becomes
> somewhat simpler as it just sweeps the list and does what it is
> told. The special ATS list, counter and locking is removed too.

OK. I'd like to give it another try. Or would you prefer to write
yourself?

Thanks
Nicolin