Re: [PATCH v4 02/16] iommu/arm-smmu-v3: Consolidate the STE generation for abort/bypass

From: Jason Gunthorpe <jgg@nvidia.com>
To: Mostafa Saleh <smostafa@google.com>
Cc: iommu@lists.linux.dev, Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	Robin Murphy <robin.murphy@arm.com>,
	Will Deacon <will@kernel.org>, Moritz Fischer <mdf@kernel.org>,
	Moritz Fischer <moritzf@google.com>,
	Michael Shavit <mshavit@google.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	patches@lists.linux.dev,
	Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Subject: Re: [PATCH v4 02/16] iommu/arm-smmu-v3: Consolidate the STE generation for abort/bypass
Date: Wed, 31 Jan 2024 10:47:02 -0400	[thread overview]
Message-ID: <20240131144702.GJ1455070@nvidia.com> (raw)
In-Reply-To: <Zbpb2JSgnRuHGIo-@google.com>

On Wed, Jan 31, 2024 at 02:40:24PM +0000, Mostafa Saleh wrote:
> > +static void arm_smmu_make_bypass_ste(struct arm_smmu_ste *target)
> > +{
> > +	memset(target, 0, sizeof(*target));
> 
> I see this can be used with the actual STE. Although this is done at init, but
> briefly making the STE abort from “arm_smmu_make_bypass_ste”, seems a bit
> fragile to me, in case we use this in the future in different scenarios, it
> might break the hitless assumption. But no strong opinion though.

At init time, when that case happens, the STE table hasn't been
installed in the HW yet. This is why that specific code path has been
directly manipulating the STE and does not call the normal update path
with the sync'ing.

It is perhaps subtle that is why it is a different flow.

(this is also why I moved the function order as it was a bit obscure
to see that indeed all this stuff was sequenced right and we were not
updating a live STE improperly)

Thanks,
Jason