linux-kernel - Re: [PATCH v4 02/13] iommu/arm-smmu-v3: Add smmu_s1_cfg to smmu

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] [day] [month] [year] [list]

Message-ID: <ZMJ9gYM+Rp8yxpv2@nvidia.com>
Date:   Thu, 27 Jul 2023 11:21:53 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Michael Shavit <mshavit@...gle.com>
Cc:     Nicolin Chen <nicolinc@...dia.com>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Joerg Roedel <joro@...tes.org>, jean-philippe@...aro.org,
        baolu.lu@...ux.intel.com, linux-arm-kernel@...ts.infradead.org,
        iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 02/13] iommu/arm-smmu-v3: Add smmu_s1_cfg to
 smmu_master

On Thu, Jul 27, 2023 at 10:04:04PM +0800, Michael Shavit wrote:
> Awesome that helps a lot. These are the patches I have in mind:
> 
> 1. Move ctx_desc out of s1_cfg
> 2. Replace domain->s1_cfg with `struct arm_smmu_ctx_desc_cfg cd_table`
> 3. Move stall_enabled from domain to arm_smmu_ctx_desc_cfg
> 4. Accept an arm_smmu_master instead of an arm_smmu_domain as the
> parameter for arm_smmu_write_ctx_desc
> --- arm_smmu_write_ctx_desc can simply get the cd_table it writes to
> from master->domain->cd_table; this get's replaced by master->cd_table
> in subsequent commits.

Makes sense

> --- SVA is weird if we cut changes here: it iterates over all masters
> a domain is attached to in order to call arm_smmu_write_ctx_desc(),
> which ends up writing to the shared cd_table since in master->domain.
> This becomes more sensible once masters stop sharing the cd _table in
> subsequent commits.

Seems like it is OK inside the series

> --- arm_smmu_write_ctx_desc is used to sync the cd for all masters the
> domain was attached to. Now that SVA is calling it in a loop, and to
> break the dependency on arm_smmu_domain, we should only sync for the
> master passed in as the parameter

Sounds correct

> 5. Introduce a skip_cd_sync parameter to arm_smmu_write_ctx_desc so
> that arm_smmu_domain_finalise_s1 can skip the sync_cd before the
> cd_table is attached to the master. Before the previous commit,
> arm_smmu_domain_finalise_s1 was calling arm_smmu_write_ctx_desc()
> before adding the master to the domain->devices list, which was used
> by arm_sync_smmu() to issue sync commands to masters. This
> optimization was broken by the previous commit since we stopped
> depending on domain->devices.

Tracking if the cd table is not yet installed in a STE might be a
cleaner approach than a flag?

> 6. Move cd_table from domain to arm_smmu_master->cd_table.

Yep

Jason