linux-kernel - Re: [PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

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: <ZFxwEYPn74Tivcrb@nvidia.com>
Date:   Thu, 11 May 2023 01:33:21 -0300
From:   Jason Gunthorpe <jgg@...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,
        nicolinc@...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 v1 0/5] Add PASID support to SMMUv3 unmanaged domains

On Thu, May 11, 2023 at 11:52:41AM +0800, Michael Shavit wrote:
> > Logically when an iommu_domain is attached to a device or a PASID a
> > STE or CD is generated from the iommu_domain's configuration but the
> > iommu_domain doesn't "hold" it
> >
> 
> Ah yes, I was using iommu domain and arm_smmu_domain interchangeably
> here since there's a 1:1 mapping between the two. In the current
> smmuv3 implementation, arm_smmu_domain holds the s1cfg structure which
> represents the s1 portion of an STE.

I mean to include the arm_smmu_domain too..

Generally this sort of seemed OK in the code, I just wouldn't use the
word 'hold' - the iommu_domain may cache a computed STE or CD value
but that the actual steering or context tables are held else where

ie you insert an iommu_domain into a steering or context table, it
does not 'hold' a table entry.

> specific to SVA about this behavior however, SVA will do the same
> amount of work whether the cd table is owned by some special iommu
> domain or by the arm_smmu_master (since we require that special iommu
> domain be attached to the master and disallow detaching it).

The CD table for SVA definately should not be part of an iommu_domain,
moving it to the master seems reasonable.
 
> Gotcha. So this patch series should be less aggressive, but is
> probably still workable with the nested domain patch series:
> 1. For (stage 1) unmanaged/dma and sva domains, arm_smmu_domains
> should hold a single CD. For the nested domain series, arm_smmu_domain
> can alternatively hold an entire s1cfg.

These are just pre computed values the can help when inserting the
iommu_domain into a steering or CD table

> 2. arm_smmu_master should own an s1cfg (which holds a cdtable) that is
> used by unmanaged/dma and sva domains attached to this master.

The arm_smmu_master's cd table can be inserted into a steering table

> 3. arm_smmu_master also holds a pointer to the live s1cfg, which may
> either points to its owned s1cfg, or the nested domain's s1cfg, or
> null (bypass or stage2)

The steering table either points to the CD table owned by the
arm_smmu_master, a S1 domain held by an iommu_domain, or a S1 & CD
table owned by userspace represented by a special nested iommu_domain
and its internal parent.

If a kernel owned S2 it attached then the S1 points at the CD table
owned by the arm_smmu_master and the CD table points to the S2, same
as if there was PASID (IIRC, from memory I don't have the spec here
right now)

Think about it in terms of what object owns the table and what other
object(s) are inserted into the the table. Nothing "holds" a table
entry.

Jason