All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"Rodel, Jorg" <jroedel@suse.de>,
	Qian Cai <quic_qiancai@quicinc.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>
Subject: Re: [PATCH v2] iommu: iommu_group_claim_dma_owner() must always assign a domain
Date: Mon, 9 May 2022 14:26:54 -0300	[thread overview]
Message-ID: <20220509172654.GP49344@nvidia.com> (raw)
In-Reply-To: <0f838b34-460c-ed83-7b98-6cda444b10c2@arm.com>

On Mon, May 09, 2022 at 10:59:11AM +0100, Robin Murphy wrote:

> IOMMU_DOMAIN_DMA_FQ now effectively takes over the original
> semantics of IOMMU_DOMAIN_DMA as the one that depends on
> driver-specific functionality.

If I grasp the FQ stuff right, it seems that this only requires the
driver to implement ops->flush_iotlb_all()? I don't see anything
obvious in any driver that is specifically _FQ related?

If yes, it makes me wonder why I see drivers implementing
ops->flush_iotlb_all() but not supporting the _FQ domain during alloc?

Further, if yes, wouldn't it make sense to just trigger FQ based on
domain->op->flush_iotlb_all being set?

It seems like there is some confusion here, because I see the sysfs
default domain store path just does this:

	/* We can bring up a flush queue without tearing down the domain */
	if (type == IOMMU_DOMAIN_DMA_FQ && prev_dom->type == IOMMU_DOMAIN_DMA) {
		ret = iommu_dma_init_fq(prev_dom);
		if (!ret)
			prev_dom->type = IOMMU_DOMAIN_DMA_FQ;
		goto out;
	}

Which will allow a driver that rejected creating DMA_FQ during alloc
to end up with DMA_FQ anyhow???

> FWIW, mtk-iommu doesn't really have any need to reject
> IOMMU_DOMAIN_UNMANAGED, they just don't have any relevant client drivers
> that want it;

Ok..

> however arm-smmu definitely does need to continue rejecting
> IOMMU_DOMAIN_DMA when it can't rely on the DT code ensuring the
> correct probe ordering with the old DT binding (otherwise client
> drivers are liable to get broken DMA ops).

I saw this code and wondered what it does?

smmu alloc_domain returns NULL, which if I read everything right
causes NULL to become the default_domain.

But then what happens? This driver has no detach_dev so after, say
VFIO does stuff, how do we get back into whatever boot-time state NULL
represents?

Is it OK because dev_iommu_priv_get() in arm_smmu_attach_dev() will
always fail if legacy? If that is the case then why allow allocating
any domain at all?

It feels like this really wants a 'IOMMU_DOMAIN_PLATFORM_DMA_OPS' set
as the default_domain meaning that when that domain is assigned, the
platform's DMA ops are handling the iommu? If I get it right..

Anyhow, thanks, this sort of helps confirm my feeling that the domain
types are a little crufty..

Thanks,
Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2022-05-09 17:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-04 19:08 [PATCH v2] iommu: iommu_group_claim_dma_owner() must always assign a domain Jason Gunthorpe via iommu
2022-05-05 10:56 ` Tian, Kevin
2022-05-05 15:33   ` Jason Gunthorpe via iommu
2022-05-05 18:56     ` Robin Murphy
2022-05-05 19:27       ` Jason Gunthorpe via iommu
2022-05-05 20:07         ` Robin Murphy
2022-05-06  9:41         ` Baolu Lu
2022-05-05 23:28     ` Tian, Kevin
2022-05-06  9:32       ` Robin Murphy
2022-05-06 13:10         ` Jason Gunthorpe via iommu
2022-05-06 13:35           ` Robin Murphy
2022-05-06 13:54             ` Jason Gunthorpe via iommu
2022-05-09  9:59               ` Robin Murphy
2022-05-09 17:26                 ` Jason Gunthorpe via iommu [this message]
2022-05-09 22:06                   ` Robin Murphy
2022-05-09 23:35                     ` Jason Gunthorpe via iommu

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=20220509172654.GP49344@nvidia.com \
    --to=iommu@lists.linux-foundation.org \
    --cc=jgg@nvidia.com \
    --cc=jroedel@suse.de \
    --cc=kevin.tian@intel.com \
    --cc=quic_qiancai@quicinc.com \
    --cc=robin.murphy@arm.com \
    --cc=will@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.