linux-kernel - Re: [PATCH v2] iommu/arm-smmu-v3: Allow default substream bypass with a pasid support

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: <ZN5Kt4mEGp0XnGVI@nvidia.com>
Date:   Thu, 17 Aug 2023 13:28:39 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Nicolin Chen <nicolinc@...dia.com>, will@...nel.org,
        joro@...tes.org, mshavit@...gle.com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev
Subject: Re: [PATCH v2] iommu/arm-smmu-v3: Allow default substream bypass
 with a pasid support

On Thu, Aug 17, 2023 at 05:24:51PM +0100, Robin Murphy wrote:
> > > @@ -2435,6 +2440,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> > >   	} else if (smmu_domain->smmu != smmu)
> > >   		ret = -EINVAL;
> > > +	/*
> > > +	 * When attaching an IDENTITY domain to a master with pasid capability,
> > > +	 * the master can still enable SVA feature by allocating a multi-entry
> > > +	 * CD table and attaching the IDENTITY domain to its default substream
> > > +	 * that alone can be byassed using the S1DSS field of the STE.
> > > +	 */
> > > +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS && master->ssid_bits &&
> > > +	    smmu->features & ARM_SMMU_FEAT_TRANS_S1)
> > > +		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS_S1DSS;
> > 
> > Then you don't technically need to do this.
> > 
> > Though if we can't atomically change the STE from IDENTITY to IDENTIY
> > with a CD then you still have to do something here,
> 
> Strictly I think we are safe to do that - fill in all the S1* fields while
> Config[0] is still 0 and they're ignored, sync, then set Config[0]. Adding a
> CD table under a translation domain should be achievable as well, since
> S1CDMax, S1ContextPtr and S1Fmt can all be updated together atomically
> (although it's still the kind of switcheroo where I'd be scared of a massive
> boulder suddenly rolling out of the ceiling...)

That sounds pretty good then we don't have to force staying in CD mode

> > but really what we
> > want is to force a CD table for all cases if PASID is enabled, and
> > force DMA domains to be S2 domains as well.
> 
> Wut? No, DMA domains really want to be stage 1, for many reasons.
> Implementing them with stage 2 when stage 1 isn't supported was always a bit
> of a bodge, but thankfully I'm not aware of anyone ever building a
> stage-2-only SMMUv3 anyway.
> 
> The most glaringly obvious one, though, is that I think people like PASID
> support and SVA to actually work ;)

Blah, I keep doing this and swapping S1/S2 in this confusing naming
scheme (and it doesn't help that AMD is backwards, sigh) - I ment what
you said :)

Thanks,
Jason