linux-kernel - Re: [PATCH v2 06/11] iommu/arm-smmu-v3: Introduce arm_smmu_s2_parent_tlb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z/69PTDANYagKX3d@Asurada-Nvidia>
Date: Tue, 15 Apr 2025 13:10:37 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <will@...nel.org>, <robin.murphy@....com>, <joro@...tes.org>,
	<kevin.tian@...el.com>, <praan@...gle.com>, <nathan@...nel.org>,
	<yi.l.liu@...el.com>, <peterz@...radead.org>, <mshavit@...gle.com>,
	<jsnitsel@...hat.com>, <smostafa@...gle.com>,
	<jeff.johnson@....qualcomm.com>, <zhangzekun11@...wei.com>,
	<linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <shameerali.kolothum.thodi@...wei.com>
Subject: Re: [PATCH v2 06/11] iommu/arm-smmu-v3: Introduce
 arm_smmu_s2_parent_tlb_ invalidation helpers

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