LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Will Deacon <will@kernel.org>
Cc: joro@8bytes.org, iommu@lists.linux-foundation.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, suravee.suthikulpanit@amd.com,
	baolu.lu@linux.intel.com, john.garry@huawei.com,
	dianders@chromium.org
Subject: Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
Date: Mon, 2 Aug 2021 15:15:50 +0100	[thread overview]
Message-ID: <3dd7cdae-7111-6ff2-6350-a0e19fe4ab66@arm.com> (raw)
In-Reply-To: <20210802130438.GA28547@willie-the-truck>

On 2021-08-02 14:04, Will Deacon wrote:
> On Wed, Jul 28, 2021 at 04:58:44PM +0100, Robin Murphy wrote:
>> To make io-pgtable aware of a flush queue being dynamically enabled,
>> allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
>> attached to, and hook up the final piece of the puzzle in iommu-dma.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>>   drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 +++++++++++
>>   drivers/iommu/dma-iommu.c                   |  3 +++
>>   3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index 19400826eba7..40fa9cb382c3 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
>>   	return ret;
>>   }
>>   
>> +static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
>> +		unsigned long quirks)
>> +{
>> +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>> +
>> +	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
>> +		struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
>> +
>> +		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>> +		return 0;
>> +	}
>> +	return -EINVAL;
>> +}
> 
> I don't see anything serialising this against a concurrent iommu_unmap(), so
> the ordering and atomicity looks quite suspicious to me here. I don't think
> it's just the page-table quirks either, but also setting cookie->fq_domain.

Heh, I confess to very much taking the cheeky "let's say nothing and see 
what Will thinks about concurrency" approach here :)

The beauty of only allowing relaxation in the strict->non-strict 
direction is that it shouldn't need serialising as such - it doesn't 
matter if the update to cookie->fq_domain is observed between 
iommu_unmap() and iommu_dma_free_iova(), since there's no correctness 
impact to queueing IOVAs which may already have been invalidated and may 
or may not have been synced. AFAICS the only condition which matters is 
that the setting of the io-pgtable quirk must observe fq_domain being 
set. It feels like there must be enough dependencies on the read side, 
but we might need an smp_wmb() between the two in iommu_dma_init_fq()?

I've also flip-flopped a bit on whether fq_domain needs to be accessed 
with READ_ONCE/WRITE_ONCE - by the time of posting I'd convinced myself 
that it was probably OK, but looking again now I suppose this wacky 
reordering is theoretically possible:


	iommu_dma_unmap() {
		bool free_fq = cookie->fq_domain; // == false

		iommu_unmap();

		if (!cookie->fq_domain) // observes new non-NULL value
			iommu_tlb_sync(); // skipped

		iommu_dma_free_iova { // inlined
			if (free_fq) // false
				queue_iova();
			else
				free_iova_fast(); // Uh-oh!
		}
	}

so although I still can't see atomicity being a problem I guess we do 
need it for the sake of reordering at least.

Cheers,
Robin.

  reply	other threads:[~2021-08-02 14:17 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
2021-07-30  6:06   ` Lu Baolu
2021-07-30  9:32   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 02/24] iommu/amd: Drop IOVA cookie management Robin Murphy
2021-07-28 15:58 ` [PATCH v2 03/24] iommu/arm-smmu: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 04/24] iommu/vt-d: " Robin Murphy
2021-07-30  6:07   ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 05/24] iommu/exynos: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 06/24] iommu/ipmmu-vmsa: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 07/24] iommu/mtk: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 08/24] iommu/rockchip: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 09/24] iommu/sprd: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 10/24] iommu/sun50i: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 11/24] iommu/virtio: " Robin Murphy
2021-07-30  9:20   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 12/24] iommu/dma: Unexport " Robin Murphy
2021-07-30  6:07   ` Lu Baolu
2021-07-30  9:21   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks Robin Murphy
2021-07-30  6:08   ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains Robin Murphy
2021-07-30  6:08   ` Lu Baolu
2021-07-30  9:23   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 15/24] iommu/amd: Prepare for multiple DMA domain types Robin Murphy
2021-07-28 15:58 ` [PATCH v2 16/24] iommu/arm-smmu: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 17/24] iommu/vt-d: " Robin Murphy
2021-07-30  6:09   ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 18/24] iommu: Express DMA strictness via the domain type Robin Murphy
2021-07-29  7:13   ` Lu Baolu
2021-07-29  9:36     ` Robin Murphy
2021-07-29 12:42       ` Lu Baolu
2021-07-30  6:09       ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
2021-07-30  6:10   ` Lu Baolu
2021-07-30  9:28   ` Jean-Philippe Brucker
2021-07-30 10:20   ` John Garry
2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
2021-07-30  6:10   ` Lu Baolu
2021-07-30  9:29   ` Jean-Philippe Brucker
2021-07-30  9:33   ` John Garry
2021-07-28 15:58 ` [PATCH v2 21/24] iommu/dma: Factor out flush queue init Robin Murphy
2021-07-30  6:11   ` Lu Baolu
2021-07-30  9:20   ` John Garry
2021-07-28 15:58 ` [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically Robin Murphy
2021-07-30  6:11   ` Lu Baolu
2021-07-30  9:24   ` John Garry
2021-07-28 15:58 ` [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface Robin Murphy
2021-08-02 13:04   ` Will Deacon
2021-08-02 14:15     ` Robin Murphy [this message]
2021-08-03 10:36       ` Will Deacon
2021-08-03 12:13         ` Robin Murphy
2021-08-03 12:35           ` Will Deacon
2021-07-28 15:58 ` [PATCH v2 24/24] iommu: Only log strictness for DMA domains Robin Murphy
2021-07-29  9:04   ` John Garry
2021-07-30  6:12   ` Lu Baolu
2021-07-29  2:55 ` [PATCH v2 00/24] iommu: Refactor DMA domain strictness chenxiang (M)
2021-07-29 10:59   ` Robin Murphy
2021-07-30  1:21     ` chenxiang (M)
2021-07-29 15:04 ` Heiko Stübner
2021-07-29 15:43   ` Robin Murphy
2021-07-29 15:53     ` Heiko Stübner
2021-07-29 16:29       ` Robin Murphy
2021-07-29 22:33 ` Doug Anderson
2021-07-30  0:06   ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3dd7cdae-7111-6ff2-6350-a0e19fe4ab66@arm.com \
    --to=robin.murphy@arm.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=dianders@chromium.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=john.garry@huawei.com \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=will@kernel.org \
    --subject='Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).