* [PATCH v2] iommu: Streamline iommu_iova_to_phys()
@ 2021-07-15 13:04 Robin Murphy
2021-07-15 14:07 ` Christoph Hellwig
2021-07-26 11:38 ` Joerg Roedel
0 siblings, 2 replies; 6+ messages in thread
From: Robin Murphy @ 2021-07-15 13:04 UTC (permalink / raw)
To: joro; +Cc: iommu, will, linux-arm-kernel
If people are going to insist on calling iommu_iova_to_phys()
pointlessly and expecting it to work, we can at least do ourselves a
favour by handling those cases in the core code, rather than repeatedly
across an inconsistent handful of drivers.
Since all the existing drivers implement the internal callback, and any
future ones are likely to want to work with iommu-dma which relies on
iova_to_phys a fair bit, we may as well remove that currently-redundant
check as well and consider it mandatory.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v2: Lowered the priority of the ops check per Baolu's suggestion, only
even further to the point of non-existence :)
---
drivers/iommu/amd/io_pgtable.c | 3 ---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 ---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ---
drivers/iommu/iommu.c | 5 ++++-
4 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
index bb0ee5c9fde7..182c93a43efd 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -493,9 +493,6 @@ static phys_addr_t iommu_v1_iova_to_phys(struct io_pgtable_ops *ops, unsigned lo
unsigned long offset_mask, pte_pgsize;
u64 *pte, __pte;
- if (pgtable->mode == PAGE_MODE_NONE)
- return iova;
-
pte = fetch_pte(pgtable, iova, &pte_pgsize);
if (!pte || !IOMMU_PTE_PRESENT(*pte))
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 3e87a9cf6da3..c9fdd0d097be 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2481,9 +2481,6 @@ arm_smmu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
- if (domain->type == IOMMU_DOMAIN_IDENTITY)
- return iova;
-
if (!ops)
return 0;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0f181f76c31b..0d04eafa3fdb 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1317,9 +1317,6 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
- if (domain->type == IOMMU_DOMAIN_IDENTITY)
- return iova;
-
if (!ops)
return 0;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 43a2041d9629..d4ba324fb0bc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2371,7 +2371,10 @@ EXPORT_SYMBOL_GPL(iommu_detach_group);
phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
{
- if (unlikely(domain->ops->iova_to_phys == NULL))
+ if (domain->type == IOMMU_DOMAIN_IDENTITY)
+ return iova;
+
+ if (domain->type == IOMMU_DOMAIN_BLOCKED)
return 0;
return domain->ops->iova_to_phys(domain, iova);
--
2.25.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
2021-07-15 13:04 [PATCH v2] iommu: Streamline iommu_iova_to_phys() Robin Murphy
@ 2021-07-15 14:07 ` Christoph Hellwig
2021-07-15 14:16 ` Robin Murphy
2021-07-26 11:38 ` Joerg Roedel
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2021-07-15 14:07 UTC (permalink / raw)
To: Robin Murphy; +Cc: will, iommu, linux-arm-kernel
On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote:
> If people are going to insist on calling iommu_iova_to_phys()
> pointlessly and expecting it to work,
Maybe we need to fix that?
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
2021-07-15 14:07 ` Christoph Hellwig
@ 2021-07-15 14:16 ` Robin Murphy
2021-07-16 6:19 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2021-07-15 14:16 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: iommu, will, linux-arm-kernel
On 2021-07-15 15:07, Christoph Hellwig wrote:
> On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote:
>> If people are going to insist on calling iommu_iova_to_phys()
>> pointlessly and expecting it to work,
>
> Maybe we need to fix that?
Feel free to try, but we didn't have much luck pushing back on it
previously, so playing whack-a-mole against netdev now is a game I'm
personally happy to stay away from ;)
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
2021-07-15 14:16 ` Robin Murphy
@ 2021-07-16 6:19 ` Christoph Hellwig
2021-07-16 12:01 ` Robin Murphy
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2021-07-16 6:19 UTC (permalink / raw)
To: Robin Murphy; +Cc: Christoph Hellwig, iommu, will, linux-arm-kernel
On Thu, Jul 15, 2021 at 03:16:08PM +0100, Robin Murphy wrote:
> On 2021-07-15 15:07, Christoph Hellwig wrote:
> > On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote:
> > > If people are going to insist on calling iommu_iova_to_phys()
> > > pointlessly and expecting it to work,
> >
> > Maybe we need to fix that?
>
> Feel free to try, but we didn't have much luck pushing back on it
> previously, so playing whack-a-mole against netdev now is a game I'm
> personally happy to stay away from ;)
One thing I've done with symbols I want people to not use it to
unexport them. But what about vfio?
While we're talking about iommu_iova_to_phys: __iommu_dma_unmap_swiotlb
calls it unconditionally, despite only needed ing the physical address.
Can we optimize that somehow by splitting out the bounce buffering case
out?
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
2021-07-16 6:19 ` Christoph Hellwig
@ 2021-07-16 12:01 ` Robin Murphy
0 siblings, 0 replies; 6+ messages in thread
From: Robin Murphy @ 2021-07-16 12:01 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: iommu, will, linux-arm-kernel
On 2021-07-16 07:19, Christoph Hellwig wrote:
> On Thu, Jul 15, 2021 at 03:16:08PM +0100, Robin Murphy wrote:
>> On 2021-07-15 15:07, Christoph Hellwig wrote:
>>> On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote:
>>>> If people are going to insist on calling iommu_iova_to_phys()
>>>> pointlessly and expecting it to work,
>>>
>>> Maybe we need to fix that?
>>
>> Feel free to try, but we didn't have much luck pushing back on it
>> previously, so playing whack-a-mole against netdev now is a game I'm
>> personally happy to stay away from ;)
>
> One thing I've done with symbols I want people to not use it to
> unexport them. But what about vfio?
Yeah, it's not like they shouldn't be calling it at all - I see it as
primarily intended for use by drivers managing their own domains, but I
don't entirely disagree with using it on DMA domains either in niche
cases - it's that they blindly grab the default domain without even
checking whether DMA mappings are actually translated or not (and thus
whether they even need to make that call every time they pull a
descriptor back out of a ringbuffer). IIRC the argument was essentially
that checking the domain type was an IOMMU API detail that those driver
shouldn't have to know about and the abstraction should just take care
of it, despite the fact that they're punching through 2 layers of
abstraction to even reach that point. And apparently keeping track of
their own descriptor addresses would be too much work, but expensive
indirect calls to either return the address they already have or go off
and do a software table walk with atomic synchronisation and everything
are fine :/
> While we're talking about iommu_iova_to_phys: __iommu_dma_unmap_swiotlb
> calls it unconditionally, despite only needed ing the physical address.
> Can we optimize that somehow by splitting out the bounce buffering case
> out?
Indeed, as I think I mentioned recently on another thread, all the
bounce-buffering stuff is fairly ugly because it's basically the old
intel-iommu code dropped in with as few changes as possible for ease of
review, since Tom was no longer able to spend time refining it, and
nobody else has got round to cleaning it up yet either. In fact the
whole flow through iommu_dma_unmap_page() flow might be the worst-hit -
reusing the iommu_dma_sync op made perfect sense when it was just cache
maintenance, but now means that at worst we do iova_to_phys *twice* plus
a pointless swiotlb_sync :(
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
2021-07-15 13:04 [PATCH v2] iommu: Streamline iommu_iova_to_phys() Robin Murphy
2021-07-15 14:07 ` Christoph Hellwig
@ 2021-07-26 11:38 ` Joerg Roedel
1 sibling, 0 replies; 6+ messages in thread
From: Joerg Roedel @ 2021-07-26 11:38 UTC (permalink / raw)
To: Robin Murphy; +Cc: iommu, will, linux-arm-kernel
On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote:
> ---
>
> v2: Lowered the priority of the ops check per Baolu's suggestion, only
> even further to the point of non-existence :)
Err, missed this newer version. Replaced the previous patch with this
one.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-26 11:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 13:04 [PATCH v2] iommu: Streamline iommu_iova_to_phys() Robin Murphy
2021-07-15 14:07 ` Christoph Hellwig
2021-07-15 14:16 ` Robin Murphy
2021-07-16 6:19 ` Christoph Hellwig
2021-07-16 12:01 ` Robin Murphy
2021-07-26 11:38 ` Joerg Roedel
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).