linux-kernel - Re: [PATCH v4 07/13] iommu/arm-smmu-v3: Keep track of attached ssids

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <ZLHp0r1g1lroGYlN@Asurada-Nvidia>
Date:   Fri, 14 Jul 2023 17:35:30 -0700
From:   Nicolin Chen <nicolinc@...dia.com>
To:     Michael Shavit <mshavit@...gle.com>
CC:     Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
        "Joerg Roedel" <joro@...tes.org>, <jean-philippe@...aro.org>,
        <jgg@...dia.com>, <baolu.lu@...ux.intel.com>,
        <linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 07/13] iommu/arm-smmu-v3: Keep track of attached ssids

On Fri, Jul 14, 2023 at 05:30:42PM +0800, Michael Shavit wrote:
 
> > And I don't quite get this part. Prior to this change, it issues
> > one ATC_INV command covering all ATC entries per comments inside
> > arm_smmu_atc_inv_to_cmd(). But now we replace that single command
> > with all attached subdomains in the list? Any reason for such a
> > change here?
> 
> Because we don't necessarily want to invalidate all PASID-domains
> attached to a master. If arm_smmu_atc_inv_domain() is called on a
> domain that is only attached with Pasid, we can restrict the
> invalidations to those specific PASID by looping over them.
>
> But yeah,
> you're right that we could potentially optimize this?
> * Skip the per-pasid invalidations if the domain is also attached to
> this master without PASID as we have to invalidate all its pasids in
> that case anyways. It's hard to imagine clients attaching a domain
> both with pasid and without pasid to the same device but could be
> possible.
> * Always invalidate all pasids by issuing atc invalidations on SSID 0.
> This sounds like the wrong trade-off??

Well, firstly it's kinda odd to have this optimization hidden in
a big rework patch. And I am not sure if it alone would work for
all use cases, as it impacts the arm_smmu_atc_inv_domain() that
passes in a zero ssid, in which case the affected function is not
used by a pasid case all the time:
	/*
	 * ATS and PASID:
...
	 * When using STRTAB_STE_1_S1DSS_SSID0 (reserving CD 0 for non-PASID
	 * traffic), translation requests without PASID create ATC entries
	 * without PASID, which must be invalidated with substream_valid clear.
	 * This has the unpleasant side-effect of invalidating all PASID-tagged
	 * ATC entries within the address range.
	 */

Thanks
Nicolin