Re: [PATCH v3 5/5] dma-iommu: account for min_align_mask
On Thu, Aug 12, 2021 at 4:12 AM Robin Murphy wrote:
>
> On 2021-08-11 03:42, David Stevens wrote:
> > From: David Stevens
> >
> > For devices which set min_align_mask, swiotlb preserves the offset of
> > the original physical address within that mask. Since __iommu_dma_map
> > accounts for non-aligned addresses, passing a non-aligned swiotlb
> > address with the swiotlb aligned size results in the offset being
> > accounted for twice in the size passed to iommu_map_atomic. The extra
> > page exposed to DMA is also not cleaned up by __iommu_dma_unmap, since
> > tht at function unmaps with the correct size. This causes mapping failures
> > if the iova gets reused, due to collisions in the iommu page tables.
> >
> > To fix this, pass the original size to __iommu_dma_map, since that
> > function already handles alignment.
> >
> > Additionally, when swiotlb returns non-aligned addresses, there is
> > padding at the start of the bounce buffer that needs to be cleared.
> >
> > Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
> > Signed-off-by: David Stevens
> > ---
> > drivers/iommu/dma-iommu.c | 23 ---
> > 1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index 89b689bf801f..ffa7e8ef5db4 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -549,9 +549,8 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device
> > *dev, phys_addr_t phys,
> > struct iommu_domain *domain = iommu_get_dma_domain(dev);
> > struct iommu_dma_cookie *cookie = domain->iova_cookie;
> > struct iova_domain *iovad = >iovad;
> > - size_t aligned_size = org_size;
> > - void *padding_start;
> > - size_t padding_size;
> > + void *tlb_start;
> > + size_t aligned_size, iova_off, mapping_end_off;
> > dma_addr_t iova;
> >
> > /*
> > @@ -566,24 +565,26 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct
> > device *dev, phys_addr_t phys,
> > if (phys == DMA_MAPPING_ERROR)
> > return DMA_MAPPING_ERROR;
> >
> > - /* Cleanup the padding area. */
> > - padding_start = phys_to_virt(phys);
> > - padding_size = aligned_size;
> > + iova_off = iova_offset(iovad, phys);
> > + tlb_start = phys_to_virt(phys - iova_off);
> >
> > + /* Cleanup the padding area. */
> > if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> > (dir == DMA_TO_DEVICE ||
> >dir == DMA_BIDIRECTIONAL)) {
> > - padding_start += org_size;
> > - padding_size -= org_size;
> > + mapping_end_off = iova_off + org_size;
> > + memset(tlb_start, 0, iova_off);
> > + memset(tlb_start + mapping_end_off, 0,
> > +aligned_size - mapping_end_off);
> > + } else {
> > + memset(tlb_start, 0, aligned_size);
> > }
> > -
> > - memset(padding_start, 0, padding_size);
> > }
> >
> > if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> > arch_sync_dma_for_device(phys, org_size, dir);
> >
> > - iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
> > + iova = __iommu_dma_map(dev, phys, org_size, prot, dma_mask);
>
> This doesn't feel right - what if the IOVA granule was equal to or
> smaller than min_align_mask, wouldn't you potentially end up mapping the
> padding rather than the data?
The phys value returned by swiotlb_tbl_map_single is the address of
the start of the data in the swiotlb buffer, so the range that needs
to be mapped is [phys, phys + org_size). __iommu_dma_map will handle
this the same as it handles a misaligned mapping in the non-swiotlb
case. It might map memory before/after the desired range, but it will
map the entire range and iova will be the mapped address of phys. Is
there something I'm missing there?
That said, considering that memory before phys might be mapped, I
think there is actually still a bug. The buffer allocated by swiotlb
needs to be aligned to the granule size to ensure that preceding
swiotlb slots aren't mapped. The swiotlb does align allocations larger
than a page to PAGE_SIZE, but if IO_TLB_SIZE < IOVA granule <
PAGE_SIZE, then there can be problems. That can't happen if PAGE_SIZE
is 4k, but it can for larger page sizes. I'll add a fix for that to
the next version of this series.
-David
> Robin.
>
> > if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
> > swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
> > return iova;
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/3] iommu/dma-iommu: Support iovad->granule > PAGE_SIZE
On Tue, Aug 10, 2021, at 11:51, Robin Murphy wrote:
> On 2021-08-09 21:45, Sven Peter wrote:
> >
> >
> > On Mon, Aug 9, 2021, at 19:41, Robin Murphy wrote:
> >> On 2021-08-07 12:47, Sven Peter via iommu wrote:
> >>>
> >>>
> >>> On Fri, Aug 6, 2021, at 20:04, Robin Murphy wrote:
> On 2021-08-06 16:55, Sven Peter via iommu wrote:
> > @@ -1006,6 +1019,31 @@ static int iommu_dma_map_sg(struct device *dev,
> > struct scatterlist *sg,
> > if (dev_is_untrusted(dev))
> > return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir,
> > attrs);
> >
> > + /*
> > +* If the IOMMU pagesize is larger than the CPU pagesize we will
> > +* very likely run into sgs with a physical address that is not
> > aligned
> > +* to an IOMMU page boundary. Fall back to just mapping every
> > entry
> > +* independently with __iommu_dma_map then.
>
> Scatterlist segments often don't have nicely aligned ends, which is why
> we already align things to IOVA granules in main loop here. I think in
> principle we'd just need to move the non-IOVA-aligned part of the
> address from sg->page to sg->offset in the temporary transformation for
> the rest of the assumptions to hold. I don't blame you for being timid
> about touching that, though - it took me 3 tries to get right when I
> first wrote it...
>
> >>>
> >>>
> >>> I've spent some time with that code now and I think we cannot use it
> >>> but have to fall back to iommu_dma_map_sg_swiotlb (even though that
> >>> swiotlb
> >>> part is a lie then):
> >>>
> >>> When we have sg_phys(s) = 0x802e65000 with s->offset = 0 the paddr
> >>> is aligned to PAGE_SIZE but has an offset of 0x1000 from something
> >>> the IOMMU can map.
> >>> Now this would result in s->offset = -0x1000 which is already weird
> >>> enough.
> >>> Offset is unsigned (and 32bit) so this will actually look like
> >>> s->offset = 0xf000 then, which isn't much better.
> >>> And then sg_phys(s) = 0x902e64000 (instead of 0x802e64000) and
> >>> we'll map some random memory in iommu_map_sg_atomic and a little bit later
> >>> everything explodes.
> >>>
> >>> Now I could probably adjust the phys addr backwards and make sure offset
> >>> is
> >>> always positive (and possibly larger than PAGE_SIZE) and later restore it
> >>> in __finalise_sg then but I feel like that's pushing this a little bit
> >>> too far.
> >>
> >> Yes, that's what I meant. At a quick guess, something like the
> >> completely untested diff below.
> >
> > That unfortunately results in unaligned mappings
>
> You mean it even compiles!? :D
I was more impressed that it already almost worked correctly :)
>
> > [9.630334] iommu: unaligned: iova 0xbff4 pa 0x000801a3b000 size
> > 0x4000 min_pagesz 0x4000
> >
> > I'll take a closer look later this week and see if I can fix it.
>
> On reflection, "s->offset ^ s_iova_off" is definitely wrong, that more
> likely wants to be "s->offset & ~s_iova_off".
>
> Robin.
>
If I change
sg_set_page(s, phys_to_page(sg_phys(s)), s_length,
s_iova_off & ~PAGE_MASK);
in __finalise_sg (and the same thing in __invalidate_sg) to
sg_set_page(s, phys_to_page(sg_phys(s) + s_iova_off), s_length,
s_iova_off & ~PAGE_MASK);
then it also restores the original fields correctly.
What is the proper way to credit you for coming up with this?
Do you create the commit and I apply it to my local tree and
include it in my submission once I have fixed the other
issues? Or do I create the commit and put a Suggested-by
in the message?
Either way, here's the patch that I have right now:
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7ce74476699d..ba31dc59566d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -907,8 +907,8 @@ static int __finalise_sg(struct device *dev, struct
scatterlist *sg, int nents,
unsigned int s_length = sg_dma_len(s);
unsigned int s_iova_len = s->length;
- s->offset += s_iova_off;
- s->length = s_length;
+ sg_set_page(s, phys_to_page(sg_phys(s) + s_iova_off), s_length,
+ s_iova_off & ~PAGE_MASK);
sg_dma_address(s) = DMA_MAPPING_ERROR;
sg_dma_len(s) = 0;
@@ -952,10 +952,11 @@ static void __invalidate_sg(struct scatterlist *sg, int
nents)
int i;
for_each_sg(sg, s, nents, i) {
- if (sg_dma_address(s) != DMA_MAPPING_ERROR)
- s->offset += sg_dma_address(s);
if (sg_dma_len(s))
- s->length = sg_dma_len(s);
+ sg_set_page(s,
+ phys_to_page(sg_phys(s) +
sg_dma_address(s)),
+ sg_dma_len(s),
+
Re: [PATCH v3 5/5] dma-iommu: account for min_align_mask
On 2021-08-11 03:42, David Stevens wrote:
From: David Stevens
For devices which set min_align_mask, swiotlb preserves the offset of
the original physical address within that mask. Since __iommu_dma_map
accounts for non-aligned addresses, passing a non-aligned swiotlb
address with the swiotlb aligned size results in the offset being
accounted for twice in the size passed to iommu_map_atomic. The extra
page exposed to DMA is also not cleaned up by __iommu_dma_unmap, since
tht at function unmaps with the correct size. This causes mapping failures
if the iova gets reused, due to collisions in the iommu page tables.
To fix this, pass the original size to __iommu_dma_map, since that
function already handles alignment.
Additionally, when swiotlb returns non-aligned addresses, there is
padding at the start of the bounce buffer that needs to be cleared.
Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
Signed-off-by: David Stevens
---
drivers/iommu/dma-iommu.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 89b689bf801f..ffa7e8ef5db4 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -549,9 +549,8 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device
*dev, phys_addr_t phys,
struct iommu_domain *domain = iommu_get_dma_domain(dev);
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = >iovad;
- size_t aligned_size = org_size;
- void *padding_start;
- size_t padding_size;
+ void *tlb_start;
+ size_t aligned_size, iova_off, mapping_end_off;
dma_addr_t iova;
/*
@@ -566,24 +565,26 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device
*dev, phys_addr_t phys,
if (phys == DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
- /* Cleanup the padding area. */
- padding_start = phys_to_virt(phys);
- padding_size = aligned_size;
+ iova_off = iova_offset(iovad, phys);
+ tlb_start = phys_to_virt(phys - iova_off);
+ /* Cleanup the padding area. */
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
(dir == DMA_TO_DEVICE ||
dir == DMA_BIDIRECTIONAL)) {
- padding_start += org_size;
- padding_size -= org_size;
+ mapping_end_off = iova_off + org_size;
+ memset(tlb_start, 0, iova_off);
+ memset(tlb_start + mapping_end_off, 0,
+ aligned_size - mapping_end_off);
+ } else {
+ memset(tlb_start, 0, aligned_size);
}
-
- memset(padding_start, 0, padding_size);
}
if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
arch_sync_dma_for_device(phys, org_size, dir);
- iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
+ iova = __iommu_dma_map(dev, phys, org_size, prot, dma_mask);
This doesn't feel right - what if the IOVA granule was equal to or
smaller than min_align_mask, wouldn't you potentially end up mapping the
padding rather than the data?
Robin.
if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
return iova;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/5] dma-iommu: Check CONFIG_SWIOTLB more broadly
On 2021-08-11 03:42, David Stevens wrote:
From: David Stevens
Introduce a new dev_use_swiotlb function to guard swiotlb code, instead
of overloading dev_is_untrusted. This allows CONFIG_SWIOTLB to be
checked more broadly, so the swiotlb related code can be removed more
aggressively.
Reviewed-by: Robin Murphy
Signed-off-by: David Stevens
---
drivers/iommu/dma-iommu.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index be0214b1455c..89b689bf801f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -317,6 +317,11 @@ static bool dev_is_untrusted(struct device *dev)
return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
}
+static bool dev_use_swiotlb(struct device *dev)
+{
+ return IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev);
+}
+
/**
* iommu_dma_init_domain - Initialise a DMA mapping domain
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -553,8 +558,7 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device
*dev, phys_addr_t phys,
* If both the physical buffer start address and size are
* page aligned, we don't need to use a bounce page.
*/
- if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
- iova_offset(iovad, phys | org_size)) {
+ if (dev_use_swiotlb(dev) && iova_offset(iovad, phys | org_size)) {
aligned_size = iova_align(iovad, org_size);
phys = swiotlb_tbl_map_single(dev, phys, org_size,
aligned_size, dir, attrs);
@@ -779,7 +783,7 @@ static void iommu_dma_sync_single_for_cpu(struct device
*dev,
{
phys_addr_t phys;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+ if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
@@ -795,7 +799,7 @@ static void iommu_dma_sync_single_for_device(struct device
*dev,
{
phys_addr_t phys;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+ if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
@@ -813,10 +817,10 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
struct scatterlist *sg;
int i;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+ if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
- if (dev_is_untrusted(dev))
+ if (dev_use_swiotlb(dev))
for_each_sg(sgl, sg, nelems, i)
iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
sg->length, dir);
@@ -832,10 +836,10 @@ static void iommu_dma_sync_sg_for_device(struct device
*dev,
struct scatterlist *sg;
int i;
- if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
+ if (dev_is_dma_coherent(dev) && !dev_use_swiotlb(dev))
return;
- if (dev_is_untrusted(dev))
+ if (dev_use_swiotlb(dev))
for_each_sg(sgl, sg, nelems, i)
iommu_dma_sync_single_for_device(dev,
sg_dma_address(sg),
@@ -999,7 +1003,7 @@ static int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg,
iommu_deferred_attach(dev, domain))
return 0;
- if (dev_is_untrusted(dev))
+ if (dev_use_swiotlb(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
@@ -1078,7 +1082,7 @@ static void iommu_dma_unmap_sg(struct device *dev, struct
scatterlist *sg,
attrs |= DMA_ATTR_SKIP_CPU_SYNC;
}
- if (dev_is_untrusted(dev)) {
+ if (dev_use_swiotlb(dev)) {
iommu_dma_unmap_sg_swiotlb(dev, sg, nents, dir, attrs);
return;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 3/5] dma-iommu: add SKIP_CPU_SYNC after syncing
On 2021-08-11 03:42, David Stevens wrote:
From: David Stevens
After syncing in map/unmap, add the DMA_ATTR_SKIP_CPU_SYNC flag so
anything that uses attrs later on will skip any sync work that has
already been completed. In particular, this skips copying from the
swiotlb twice during unmap.
Signed-off-by: David Stevens
---
drivers/iommu/dma-iommu.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 4f0cc4a0a61f..be0214b1455c 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -859,8 +859,11 @@ static dma_addr_t iommu_dma_map_page(struct device *dev,
struct page *page,
static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
size_t size, enum dma_data_direction dir, unsigned long attrs)
{
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
iommu_dma_sync_single_for_cpu(dev, dma_handle, size, dir);
+ attrs |= DMA_ATTR_SKIP_CPU_SYNC;
+ }
+
__iommu_dma_unmap_swiotlb(dev, dma_handle, size, dir, attrs);
Again, fold that into here and put the arch sync in an else case to the
is_swiotlb_buffer() check.
}
@@ -999,8 +1002,10 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
if (dev_is_untrusted(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
+ attrs |= DMA_ATTR_SKIP_CPU_SYNC;
+ }
Why? attrs is never referenced again after this.
/*
* Work out how much IOVA space we need, and align the segments to
@@ -1068,8 +1073,10 @@ static void iommu_dma_unmap_sg(struct device *dev,
struct scatterlist *sg,
struct scatterlist *tmp;
int i;
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
iommu_dma_sync_sg_for_cpu(dev, sg, nents, dir);
+ attrs |= DMA_ATTR_SKIP_CPU_SYNC;
+ }
Just move it down so it's out of the SWIOTLB path entirely. Exactly like
you did in patch #2 for map_sg, conspicuous in the hunk above.
Robin.
if (dev_is_untrusted(dev)) {
iommu_dma_unmap_sg_swiotlb(dev, sg, nents, dir, attrs);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/5] dma-iommu: fix arch_sync_dma for map
On 2021-08-11 03:42, David Stevens wrote:
From: David Stevens
When calling arch_sync_dma, we need to pass it the memory that's
actually being used for dma. When using swiotlb bounce buffers, this is
the bounce buffer. Move arch_sync_dma into the __iommu_dma_map_swiotlb
helper, so it can use the bounce buffer address if necessary. This also
means it is no longer necessary to call iommu_dma_sync_sg_for_device in
iommu_dma_map_sg for untrusted devices.
Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Signed-off-by: David Stevens
---
drivers/iommu/dma-iommu.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 54e103b989d9..4f0cc4a0a61f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -576,6 +576,9 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device
*dev, phys_addr_t phys,
memset(padding_start, 0, padding_size);
}
+ if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
Make that an "else if" - otherwise you're just reintroducing the same
thing that the third hunk is trying to clean up.
+ arch_sync_dma_for_device(phys, org_size, dir);
+
iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
@@ -848,14 +851,9 @@ static dma_addr_t iommu_dma_map_page(struct device *dev,
struct page *page,
{
phys_addr_t phys = page_to_phys(page) + offset;
bool coherent = dev_is_dma_coherent(dev);
- dma_addr_t dma_handle;
- dma_handle = __iommu_dma_map_swiotlb(dev, phys, size, dma_get_mask(dev),
+ return __iommu_dma_map_swiotlb(dev, phys, size, dma_get_mask(dev),
coherent, dir, attrs);
Just fold __iommu_dma_map_swiotlb() back into here and have
iommu_dma_map_sg_swiotlb() call iommu_dma_map_page() in the typical
pattern of dma-direct and others. Apparently the only purpose served by
that indirection was allowing these bugs to exist...
Robin.
- if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
- dma_handle != DMA_MAPPING_ERROR)
- arch_sync_dma_for_device(phys, size, dir);
- return dma_handle;
}
static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
@@ -998,12 +996,12 @@ static int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg,
iommu_deferred_attach(dev, domain))
return 0;
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
- iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
-
if (dev_is_untrusted(dev))
return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs);
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
+
/*
* Work out how much IOVA space we need, and align the segments to
* IOVA granules for the IOMMU driver to handle. With some clever
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/5] dma-iommu: fix sync_sg with swiotlb
On 2021-08-11 06:57, Christoph Hellwig wrote:
On Wed, Aug 11, 2021 at 11:42:43AM +0900, David Stevens wrote:
From: David Stevens
The is_swiotlb_buffer function takes the physical address of the swiotlb
buffer, not the physical address of the original buffer. The sglist
contains the physical addresses of the original buffer, so for the
sync_sg functions to work properly when a bounce buffer might have been
used, we need to use iommu_iova_to_phys to look up the physical address.
This is what sync_single does, so call that function on each sglist
segment.
The previous code mostly worked because swiotlb does the transfer on map
and unmap. However, any callers which use DMA_ATTR_SKIP_CPU_SYNC with
sglists or which call sync_sg would not have had anything copied to the
bounce buffer.
Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
Signed-off-by: David Stevens
---
drivers/iommu/dma-iommu.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..54e103b989d9 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -813,14 +813,13 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
return;
+ if (dev_is_untrusted(dev))
+ for_each_sg(sgl, sg, nelems, i)
+ iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
+ sg->length, dir);
+ else
+ for_each_sg(sgl, sg, nelems, i)
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
}
I'd remove the above check and fold the if (!dev_is_dma_coherent(dev))
into the else line. Same for iommu_dma_sync_sg_for_device.
+1
With those also cleaned up,
Reviewed-by: Robin Murphy
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv4] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
On 2021-08-11 21:23, Robin Murphy wrote:
On 2021-08-11 11:30, Will Deacon wrote:
On Wed, Aug 11, 2021 at 11:37:25AM +0530, Sai Prakash Ranjan wrote:
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f7da8953afbe..3904b598e0f9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -327,9 +327,16 @@ static void arm_smmu_tlb_inv_range_s2(unsigned
long iova, size_t size,
static void arm_smmu_tlb_inv_walk_s1(unsigned long iova, size_t
size,
size_t granule, void *cookie)
{
- arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
- ARM_SMMU_CB_S1_TLBIVA);
- arm_smmu_tlb_sync_context(cookie);
+ struct arm_smmu_domain *smmu_domain = cookie;
+ struct arm_smmu_cfg *cfg = _domain->cfg;
+
+ if (cfg->flush_walk_prefer_tlbiasid) {
+ arm_smmu_tlb_inv_context_s1(cookie);
Hmm, this introduces an unconditional wmb() if tlbiasid is preferred.
I
think that should be predicated on ARM_SMMU_FEAT_COHERENT_WALK like it
is
for the by-VA ops. Worth doing as a separate patch.
+ } else {
+ arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
+ ARM_SMMU_CB_S1_TLBIVA);
+ arm_smmu_tlb_sync_context(cookie);
+ }
}
static void arm_smmu_tlb_add_page_s1(struct iommu_iotlb_gather
*gather,
@@ -765,8 +772,10 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
+ if (!iommu_get_dma_strict(domain)) {
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ cfg->flush_walk_prefer_tlbiasid = true;
This is going to interact badly with Robin's series to allow dynamic
transition to non-strict mode, as we don't have a mechanism to switch
over to the by-ASID behaviour. Yes, it should _work_, but it's ugly
having
different TLBI behaviour just because of the how the domain became
non-strict.
Robin -- I think this originated from your idea at [1]. Any idea how
to make
it work with your other series, or shall we drop this part for now and
leave
the TLB invalidation behaviour the same for now?
Yeah, I'd say drop it - I'm currently half an hour into a first
attempt at removing io_pgtable_tlb_flush_walk() entirely, which would
make it moot for non-strict anyway.
I have dropped it and sent a v5.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCHv5] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
Currently for iommu_unmap() of large scatter-gather list with page size
elements, the majority of time is spent in flushing of partial walks in
__arm_lpae_unmap() which is a VA based TLB invalidation invalidating
page-by-page on iommus like arm-smmu-v2 (TLBIVA).
For example: to unmap a 32MB scatter-gather list with page size elements
(8192 entries), there are 16->2MB buffer unmaps based on the pgsize (2MB
for 4K granule) and each of 2MB will further result in 512 TLBIVAs (2MB/4K)
resulting in a total of 8192 TLBIVAs (512*16) for 16->2MB causing a huge
overhead.
On qcom implementation, there are several performance improvements for
TLB cache invalidations in HW like wait-for-safe (for realtime clients
such as camera and display) and few others to allow for cache
lookups/updates when TLBI is in progress for the same context bank.
So the cost of over-invalidation is less compared to the unmap latency
on several usecases like camera which deals with large buffers. So,
ASID based TLB invalidations (TLBIASID) can be used to invalidate the
entire context for partial walk flush thereby improving the unmap
latency.
For this example of 32MB scatter-gather list unmap, this change results
in just 16 ASID based TLB invalidations (TLBIASIDs) as opposed to 8192
TLBIVAs thereby increasing the performance of unmaps drastically.
Test on QTI SM8150 SoC for 10 iterations of iommu_{map_sg}/unmap:
(average over 10 iterations)
Before this optimization:
sizeiommu_map_sg iommu_unmap
4K2.067 us 1.854 us
64K9.598 us 8.802 us
1M 148.890 us 130.718 us
2M 305.864 us67.291 us
12M 1793.604 us 390.838 us
16M 2386.848 us 518.187 us
24M 3563.296 us 775.989 us
32M 4747.171 us 1033.364 us
After this optimization:
sizeiommu_map_sg iommu_unmap
4K1.723 us 1.765 us
64K9.880 us 8.869 us
1M 155.364 us 135.223 us
2M 303.906 us 5.385 us
12M 1786.557 us21.250 us
16M 2391.890 us27.437 us
24M 3570.895 us39.937 us
32M 4755.234 us51.797 us
Real world data also shows big difference in unmap performance as below:
There were reports of camera frame drops because of high overhead in
iommu unmap without this optimization because of frequent unmaps issued
by camera of about 100MB/s taking more than 100ms thereby causing frame
drops.
Signed-off-by: Sai Prakash Ranjan
---
Changes in v5:
* Drop non-strict mode change as it will conflict with Robin's series
Changes in v4:
* Use a flag in struct arm_smmu_cfg to prefer TLBIASID (Will)
Changes in v3:
* Move the logic to arm-smmu driver from io-pgtable (Robin)
* Use a new set of iommu_flush_ops->arm_smmu_s1_tlb_impl_ops and use it for
qcom impl
Changes in v2:
* Add a quirk to choose tlb_flush_all in partial walk flush
* Set the quirk for QTI SoC implementation
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 11 +++
drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++---
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 9b9d13ec5a88..55690af1b25d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -193,6 +193,8 @@ static int qcom_adreno_smmu_init_context(struct
arm_smmu_domain *smmu_domain,
{
struct adreno_smmu_priv *priv;
+ smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
+
/* Only enable split pagetables for the GPU device (SID 0) */
if (!qcom_adreno_smmu_is_gpu_device(dev))
return 0;
@@ -235,6 +237,14 @@ static const struct of_device_id
qcom_smmu_client_of_match[] __maybe_unused = {
{ }
};
+static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
+ struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
+{
+ smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
+
+ return 0;
+}
+
static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
{
unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups -
1);
@@ -358,6 +368,7 @@ static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
}
static const struct arm_smmu_impl qcom_smmu_impl = {
+ .init_context = qcom_smmu_init_context,
.cfg_probe = qcom_smmu_cfg_probe,
.def_domain_type = qcom_smmu_def_domain_type,
.reset = qcom_smmu500_reset,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f7da8953afbe..67b660b0551d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -327,9 +327,16 @@ static void
[PATCH] iommu/arm-smmu-v3: Stop pre-zeroing batch commands
Pre-zeroing the batched commands structure is inefficient, as individual
commands are zeroed later in arm_smmu_cmdq_build_cmd(). The size is quite
large and commonly most commands won't even be used:
struct arm_smmu_cmdq_batch cmds = {};
345c: 5281mov w1, #0x0// #0
3460: d2808102mov x2, #0x408 // #1032
3464: 910143a0add x0, x29, #0x50
3468: 9400bl 0
Stop pre-zeroing the complete structure and only zero the num member.
Signed-off-by: John Garry
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 cbc57edfa4e2..2a4f3fb938f6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1082,7 +1082,7 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain
*smmu_domain,
size_t i;
unsigned long flags;
struct arm_smmu_master *master;
- struct arm_smmu_cmdq_batch cmds = {};
+ struct arm_smmu_cmdq_batch cmds;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cmdq_ent cmd = {
.opcode = CMDQ_OP_CFGI_CD,
@@ -1092,6 +1092,8 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain
*smmu_domain,
},
};
+ cmds.num = 0;
+
spin_lock_irqsave(_domain->devices_lock, flags);
list_for_each_entry(master, _domain->devices, domain_head) {
for (i = 0; i < master->num_streams; i++) {
@@ -1908,7 +1910,7 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain
*smmu_domain, int ssid,
unsigned long flags;
struct arm_smmu_cmdq_ent cmd;
struct arm_smmu_master *master;
- struct arm_smmu_cmdq_batch cmds = {};
+ struct arm_smmu_cmdq_batch cmds;
if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_ATS))
return 0;
@@ -1932,6 +1934,8 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain
*smmu_domain, int ssid,
arm_smmu_atc_inv_to_cmd(ssid, iova, size, );
+ cmds.num = 0;
+
spin_lock_irqsave(_domain->devices_lock, flags);
list_for_each_entry(master, _domain->devices, domain_head) {
if (!master->ats_enabled)
@@ -1980,7 +1984,7 @@ static void __arm_smmu_tlb_inv_range(struct
arm_smmu_cmdq_ent *cmd,
struct arm_smmu_device *smmu = smmu_domain->smmu;
unsigned long end = iova + size, num_pages = 0, tg = 0;
size_t inv_range = granule;
- struct arm_smmu_cmdq_batch cmds = {};
+ struct arm_smmu_cmdq_batch cmds;
if (!size)
return;
@@ -1998,6 +2002,8 @@ static void __arm_smmu_tlb_inv_range(struct
arm_smmu_cmdq_ent *cmd,
num_pages = size >> tg;
}
+ cmds.num = 0;
+
while (iova < end) {
if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
/*
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv4] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
On 2021-08-11 11:30, Will Deacon wrote:
On Wed, Aug 11, 2021 at 11:37:25AM +0530, Sai Prakash Ranjan wrote:
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f7da8953afbe..3904b598e0f9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -327,9 +327,16 @@ static void arm_smmu_tlb_inv_range_s2(unsigned long iova,
size_t size,
static void arm_smmu_tlb_inv_walk_s1(unsigned long iova, size_t size,
size_t granule, void *cookie)
{
- arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
- ARM_SMMU_CB_S1_TLBIVA);
- arm_smmu_tlb_sync_context(cookie);
+ struct arm_smmu_domain *smmu_domain = cookie;
+ struct arm_smmu_cfg *cfg = _domain->cfg;
+
+ if (cfg->flush_walk_prefer_tlbiasid) {
+ arm_smmu_tlb_inv_context_s1(cookie);
Hmm, this introduces an unconditional wmb() if tlbiasid is preferred. I
think that should be predicated on ARM_SMMU_FEAT_COHERENT_WALK like it is
for the by-VA ops. Worth doing as a separate patch.
+ } else {
+ arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
+ ARM_SMMU_CB_S1_TLBIVA);
+ arm_smmu_tlb_sync_context(cookie);
+ }
}
static void arm_smmu_tlb_add_page_s1(struct iommu_iotlb_gather *gather,
@@ -765,8 +772,10 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
+ if (!iommu_get_dma_strict(domain)) {
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ cfg->flush_walk_prefer_tlbiasid = true;
This is going to interact badly with Robin's series to allow dynamic
transition to non-strict mode, as we don't have a mechanism to switch
over to the by-ASID behaviour. Yes, it should _work_, but it's ugly having
different TLBI behaviour just because of the how the domain became
non-strict.
Robin -- I think this originated from your idea at [1]. Any idea how to make
it work with your other series, or shall we drop this part for now and leave
the TLB invalidation behaviour the same for now?
Yeah, I'd say drop it - I'm currently half an hour into a first attempt
at removing io_pgtable_tlb_flush_walk() entirely, which would make it
moot for non-strict anyway.
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()
On 8/11/21 7:19 AM, Kirill A. Shutemov wrote:
> On Tue, Aug 10, 2021 at 02:48:54PM -0500, Tom Lendacky wrote:
>> On 8/10/21 1:45 PM, Kuppuswamy, Sathyanarayanan wrote:
>>>
>>>
>>> On 7/27/21 3:26 PM, Tom Lendacky wrote:
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index de01903c3735..cafed6456d45 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -19,7 +19,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long
physaddr,
* there is no need to zero it after changing the memory encryption
* attribute.
*/
- if (mem_encrypt_active()) {
+ if (prot_guest_has(PATTR_MEM_ENCRYPT)) {
vaddr = (unsigned long)__start_bss_decrypted;
vaddr_end = (unsigned long)__end_bss_decrypted;
>>>
>>>
>>> Since this change is specific to AMD, can you replace PATTR_MEM_ENCRYPT with
>>> prot_guest_has(PATTR_SME) || prot_guest_has(PATTR_SEV). It is not used in
>>> TDX.
>>
>> This is a direct replacement for now.
>
> With current implementation of prot_guest_has() for TDX it breaks boot for
> me.
>
> Looking at code agains, now I *think* the reason is accessing a global
> variable from __startup_64() inside TDX version of prot_guest_has().
>
> __startup_64() is special. If you access any global variable you need to
> use fixup_pointer(). See comment before __startup_64().
>
> I'm not sure how you get away with accessing sme_me_mask directly from
> there. Any clues? Maybe just a luck and complier generates code just right
> for your case, I donno.
Hmm... yeah, could be that the compiler is using rip-relative addressing
for it because it lives in the .data section?
For the static variables in mem_encrypt_identity.c I did an assembler rip
relative LEA, but probably could have passed physaddr to sme_enable() and
used a fixup_pointer() style function, instead.
>
> A separate point is that TDX version of prot_guest_has() relies on
> cpu_feature_enabled() which is not ready at this point.
Does TDX have to do anything special to make memory able to be shared with
the hypervisor? You might have to use something that is available earlier
than cpu_feature_enabled() in that case (should you eventually support
kvmclock).
>
> I think __bss_decrypted fixup has to be done if sme_me_mask is non-zero.
> Or just do it uncoditionally because it's NOP for sme_me_mask == 0.
For SNP, we'll have to additionally call the HV to update the RMP to make
the memory shared. But that could also be done unconditionally since the
early_snp_set_memory_shared() routine will check for SNP before doing
anything.
Thanks,
Tom
>
>> I think the change you're requesting
>> should be done as part of the TDX support patches so it's clear why it is
>> being changed.
>>
>> But, wouldn't TDX still need to do something with this shared/unencrypted
>> area, though? Or since it is shared, there's actually nothing you need to
>> do (the bss decrpyted section exists even if CONFIG_AMD_MEM_ENCRYPT is not
>> configured)?
>
> AFAICS, only kvmclock uses __bss_decrypted. We don't enable kvmclock in
> TDX at the moment. It may change in the future.
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/11] mm: Introduce a function to check for virtualization protection features
On 8/11/21 9:53 AM, Kuppuswamy, Sathyanarayanan wrote: > On 7/27/21 3:26 PM, Tom Lendacky wrote: >> diff --git a/include/linux/protected_guest.h >> b/include/linux/protected_guest.h >> new file mode 100644 >> index ..f8ed7b72967b >> --- /dev/null >> +++ b/include/linux/protected_guest.h >> @@ -0,0 +1,32 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Protected Guest (and Host) Capability checks >> + * >> + * Copyright (C) 2021 Advanced Micro Devices, Inc. >> + * >> + * Author: Tom Lendacky >> + */ >> + >> +#ifndef _PROTECTED_GUEST_H >> +#define _PROTECTED_GUEST_H >> + >> +#ifndef __ASSEMBLY__ > > Can you include headers for bool type and false definition? Can do. Thanks, Tom > > --- a/include/linux/protected_guest.h > +++ b/include/linux/protected_guest.h > @@ -12,6 +12,9 @@ > > #ifndef __ASSEMBLY__ > > +#include > +#include > > Otherwise, I see following errors in multi-config auto testing. > > include/linux/protected_guest.h:40:15: error: unknown type name 'bool' > include/linux/protected_guest.h:40:63: error: 'false' undeclared (first > use in this functi > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/11] mm: Introduce a function to check for virtualization protection features
On 7/27/21 3:26 PM, Tom Lendacky wrote:
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
new file mode 100644
index ..f8ed7b72967b
--- /dev/null
+++ b/include/linux/protected_guest.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Protected Guest (and Host) Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky
+ */
+
+#ifndef _PROTECTED_GUEST_H
+#define _PROTECTED_GUEST_H
+
+#ifndef __ASSEMBLY__
Can you include headers for bool type and false definition?
--- a/include/linux/protected_guest.h
+++ b/include/linux/protected_guest.h
@@ -12,6 +12,9 @@
#ifndef __ASSEMBLY__
+#include
+#include
Otherwise, I see following errors in multi-config auto testing.
include/linux/protected_guest.h:40:15: error: unknown type name 'bool'
include/linux/protected_guest.h:40:63: error: 'false' undeclared (first use in
this functi
+
+#define PATTR_MEM_ENCRYPT 0 /* Encrypted memory */
+#define PATTR_HOST_MEM_ENCRYPT 1 /* Host encrypted memory */
+#define PATTR_GUEST_MEM_ENCRYPT2 /* Guest encrypted
memory */
+#define PATTR_GUEST_PROT_STATE 3 /* Guest encrypted state */
+
+#ifdef CONFIG_ARCH_HAS_PROTECTED_GUEST
+
+#include
+
+#else /* !CONFIG_ARCH_HAS_PROTECTED_GUEST */
+
+static inline bool prot_guest_has(unsigned int attr) { return false; }
+
+#endif /* CONFIG_ARCH_HAS_PROTECTED_GUEST */
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _PROTECTED_GUEST_H */
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] iommu/vt-d: Move intel_iommu_ops to header file
On Wed, Aug 11, 2021 at 10:02:48PM +0800, Lu Baolu wrote: > On 2021/8/11 21:49, Andy Shevchenko wrote: > > On Fri, Jul 30, 2021 at 09:01:41PM +0800, Lu Baolu wrote: > > > On 2021/7/30 16:05, Andy Shevchenko wrote: > > > > On Fri, Jul 30, 2021 at 10:20:08AM +0800, Lu Baolu wrote: > > > > > On 7/30/21 12:35 AM, Andy Shevchenko wrote: > > > > > > Compiler is not happy about hidden declaration of intel_iommu_ops. > > > > > > > > > > > > .../drivers/iommu/intel/iommu.c:414:24: warning: symbol > > > > > > 'intel_iommu_ops' was not declared. Should it be static? > > > > > > > > > > > > Move declaration to header file to make compiler happy. > > > > > > > > > > Thanks for the cleanup. Sharing data structures between different > > > > > files > > > > > doesn't seem to be a good design. How about adding a helper so that > > > > > the > > > > > intel_iommu_ops could be a static one? > > > > > > > > Whatever suits the purpose. > > > > Can you apply patch 2 of this series, please? > > > > > > Yes, I will. Thanks! > > > > Gentle reminder. > > Thanks. Normally I will queue the vt-d patches to Joerg in the rc6 week. I see, but don't we need to have them in Linux Next for a few weeks for testing? Perhaps you need to add your tree to be integrated in the Linux Next? -- With Best Regards, Andy Shevchenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] iommu/vt-d: Move intel_iommu_ops to header file
On 2021/8/11 21:49, Andy Shevchenko wrote: On Fri, Jul 30, 2021 at 09:01:41PM +0800, Lu Baolu wrote: On 2021/7/30 16:05, Andy Shevchenko wrote: On Fri, Jul 30, 2021 at 10:20:08AM +0800, Lu Baolu wrote: On 7/30/21 12:35 AM, Andy Shevchenko wrote: Compiler is not happy about hidden declaration of intel_iommu_ops. .../drivers/iommu/intel/iommu.c:414:24: warning: symbol 'intel_iommu_ops' was not declared. Should it be static? Move declaration to header file to make compiler happy. Thanks for the cleanup. Sharing data structures between different files doesn't seem to be a good design. How about adding a helper so that the intel_iommu_ops could be a static one? Whatever suits the purpose. Can you apply patch 2 of this series, please? Yes, I will. Thanks! Gentle reminder. Thanks. Normally I will queue the vt-d patches to Joerg in the rc6 week. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] iommu/vt-d: Move intel_iommu_ops to header file
On Fri, Jul 30, 2021 at 09:01:41PM +0800, Lu Baolu wrote: > On 2021/7/30 16:05, Andy Shevchenko wrote: > > On Fri, Jul 30, 2021 at 10:20:08AM +0800, Lu Baolu wrote: > > > On 7/30/21 12:35 AM, Andy Shevchenko wrote: > > > > Compiler is not happy about hidden declaration of intel_iommu_ops. > > > > > > > > .../drivers/iommu/intel/iommu.c:414:24: warning: symbol > > > > 'intel_iommu_ops' was not declared. Should it be static? > > > > > > > > Move declaration to header file to make compiler happy. > > > > > > Thanks for the cleanup. Sharing data structures between different files > > > doesn't seem to be a good design. How about adding a helper so that the > > > intel_iommu_ops could be a static one? > > > > Whatever suits the purpose. > > Can you apply patch 2 of this series, please? > > > > Yes, I will. Thanks! Gentle reminder. -- With Best Regards, Andy Shevchenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 0/2] dma-pool: allow user to disable atomic pool
On 8/10/21 9:23 PM, Baoquan He wrote: > On 08/10/21 at 03:52pm, Tom Lendacky wrote: >> On 8/5/21 1:54 AM, Baoquan He wrote: >>> On 06/24/21 at 11:47am, Robin Murphy wrote: On 2021-06-24 10:29, Baoquan He wrote: > On 06/24/21 at 08:40am, Christoph Hellwig wrote: ... > Looking at the those related commits, the below one from David tells > that atomic dma pool is used when device require non-blocking and > unencrypted buffer. When I checked the system I borrowed, it's AMD EYPC > and SME is enabled. And it has many pci devices, as you can see, its 'ls > pci' outputs 113 lines. But disabling the three atomic pools didn't > trigger any error on that AMD system. Does it mean only specific devices > need this atomic pool in SME/SEV enabling case? Should we add more > details in document or code comment to make clear this? It very well could be just the devices being used. Under SME (bare metal), if a device supports 64-bit DMA, then bounce buffers aren't used and the DMA can be performed directly to encrypted memory, so there is no need to issue a set_memory_decrypted() call, so I would assume it likely isn't using the pool. Under SEV, however, all DMA has to go through guest un-encrypted memory. If you pass through a device that does dma_alloc_coherent() calls with GFP_ATOMIC, then the pool will be needed. Thanks, Tom ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 24/24] iommu: Allow enabling non-strict mode dynamically
Allocating and enabling a flush queue is in fact something we can
reasonably do while a DMA domain is active, without having to rebuild it
from scratch. Thus we can allow a strict -> non-strict transition from
sysfs without requiring to unbind the device's driver, which is of
particular interest to users who want to make selective relaxations to
critical devices like the one serving their root filesystem.
Disabling and draining a queue also seems technically possible to
achieve without rebuilding the whole domain, but would certainly be more
involved. Furthermore there's not such a clear use-case for tightening
up security *after* the device may already have done whatever it is that
you don't trust it not to do, so we only consider the relaxation case.
Signed-off-by: Robin Murphy
---
v3: Actually think about concurrency, rework most of the fq data
accesses to be (hopefully) safe and comment it all
v4: Squash in simplified iommu_dma_init_fq() refactor, clean up previous
barrier from init_iova_flush_queue(), drop rogue header change
(turns out there's a whole other rabbit hole of iova_fq cleanup to
venture down after this...)
---
drivers/iommu/dma-iommu.c | 47 ++-
drivers/iommu/iommu.c | 17 ++
drivers/iommu/iova.c | 11 -
include/linux/dma-iommu.h | 6 +
4 files changed, 57 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 01fa83229118..27f98a29f49a 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -310,6 +310,30 @@ static bool dev_is_untrusted(struct device *dev)
return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
}
+/* sysfs updates are serialised by the mutex of the group owning @domain */
+int iommu_dma_init_fq(struct iommu_domain *domain)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ int ret;
+
+ if (cookie->fq_domain)
+ return 0;
+
+ ret = init_iova_flush_queue(>iovad, iommu_dma_flush_iotlb_all,
+ iommu_dma_entry_dtor);
+ if (ret) {
+ pr_warn("iova flush queue initialization failed\n");
+ return ret;
+ }
+ /*
+* Prevent incomplete iovad->fq being observable. Pairs with path from
+* __iommu_dma_unmap() through iommu_dma_free_iova() to queue_iova()
+*/
+ smp_wmb();
+ WRITE_ONCE(cookie->fq_domain, domain);
+ return 0;
+}
+
/**
* iommu_dma_init_domain - Initialise a DMA mapping domain
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -364,15 +388,8 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
init_iova_domain(iovad, 1UL << order, base_pfn);
/* If the FQ fails we can simply fall back to strict mode */
- if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain) {
- if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
- iommu_dma_entry_dtor)) {
- pr_warn("iova flush queue initialization failed\n");
- domain->type = IOMMU_DOMAIN_DMA;
- } else {
- cookie->fq_domain = domain;
- }
- }
+ if (domain->type == IOMMU_DOMAIN_DMA_FQ && iommu_dma_init_fq(domain))
+ domain->type = IOMMU_DOMAIN_DMA;
return iova_reserve_iommu_regions(dev, domain);
}
@@ -447,17 +464,17 @@ static dma_addr_t iommu_dma_alloc_iova(struct
iommu_domain *domain,
}
static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
- dma_addr_t iova, size_t size, struct page *freelist)
+ dma_addr_t iova, size_t size, struct iommu_iotlb_gather *gather)
{
struct iova_domain *iovad = >iovad;
/* The MSI case is only ever cleaning up its most recent allocation */
if (cookie->type == IOMMU_DMA_MSI_COOKIE)
cookie->msi_iova -= size;
- else if (cookie->fq_domain) /* non-strict mode */
+ else if (gather && gather->queued)
queue_iova(iovad, iova_pfn(iovad, iova),
size >> iova_shift(iovad),
- (unsigned long)freelist);
+ (unsigned long)gather->freelist);
else
free_iova_fast(iovad, iova_pfn(iovad, iova),
size >> iova_shift(iovad));
@@ -476,14 +493,14 @@ static void __iommu_dma_unmap(struct device *dev,
dma_addr_t dma_addr,
dma_addr -= iova_off;
size = iova_align(iovad, size + iova_off);
iommu_iotlb_gather_init(_gather);
- iotlb_gather.queued = cookie->fq_domain;
+ iotlb_gather.queued = READ_ONCE(cookie->fq_domain);
unmapped = iommu_unmap_fast(domain, dma_addr, size, _gather);
WARN_ON(unmapped != size);
- if
[PATCH v4 23/24] iommu: Merge strictness and domain type configs
To parallel the sysfs behaviour, merge the new build-time option
for DMA domain strictness into the default domain type choice.
Suggested-by: Joerg Roedel
Reviewed-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
Reviewed-by: John Garry
Signed-off-by: Robin Murphy
---
v3: Remember to update parameter documentation as well
---
.../admin-guide/kernel-parameters.txt | 8 +-
drivers/iommu/Kconfig | 80 +--
drivers/iommu/iommu.c | 2 +-
3 files changed, 44 insertions(+), 46 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 90b525cf0ec2..19192b39952a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2045,11 +2045,9 @@
1 - Strict mode.
DMA unmap operations invalidate IOMMU hardware TLBs
synchronously.
- unset - Use value of CONFIG_IOMMU_DEFAULT_{LAZY,STRICT}.
- Note: on x86, the default behaviour depends on the
- equivalent driver-specific parameters, but a strict
- mode explicitly specified by either method takes
- precedence.
+ unset - Use value of
CONFIG_IOMMU_DEFAULT_DMA_{LAZY,STRICT}.
+ Note: on x86, strict mode specified via one of the
+ legacy driver-specific options takes precedence.
iommu.passthrough=
[ARM64, X86] Configure DMA to bypass the IOMMU by
default.
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c84da8205be7..6e06f876d75a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -79,55 +79,55 @@ config IOMMU_DEBUGFS
debug/iommu directory, and then populate a subdirectory with
entries as required.
-config IOMMU_DEFAULT_PASSTHROUGH
- bool "IOMMU passthrough by default"
- depends on IOMMU_API
- help
- Enable passthrough by default, removing the need to pass in
- iommu.passthrough=on or iommu=pt through command line. If this
- is enabled, you can still disable with iommu.passthrough=off
- or iommu=nopt depending on the architecture.
-
- If unsure, say N here.
-
choice
- prompt "IOMMU default DMA IOTLB invalidation mode"
- depends on IOMMU_DMA
-
- default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU)
- default IOMMU_DEFAULT_STRICT
+ prompt "IOMMU default domain type"
+ depends on IOMMU_API
+ default IOMMU_DEFAULT_DMA_LAZY if AMD_IOMMU || INTEL_IOMMU
+ default IOMMU_DEFAULT_DMA_STRICT
help
- This option allows an IOMMU DMA IOTLB invalidation mode to be
- chosen at build time, to override the default mode of each ARCH,
- removing the need to pass in kernel parameters through command line.
- It is still possible to provide common boot params to override this
- config.
+ Choose the type of IOMMU domain used to manage DMA API usage by
+ device drivers. The options here typically represent different
+ levels of tradeoff between robustness/security and performance,
+ depending on the IOMMU driver. Not all IOMMUs support all options.
+ This choice can be overridden at boot via the command line, and for
+ some devices also at runtime via sysfs.
If unsure, keep the default.
-config IOMMU_DEFAULT_STRICT
- bool "strict"
+config IOMMU_DEFAULT_DMA_STRICT
+ bool "Translated - Strict"
help
- For every IOMMU DMA unmap operation, the flush operation of IOTLB and
- the free operation of IOVA are guaranteed to be done in the unmap
- function.
+ Trusted devices use translation to restrict their access to only
+ DMA-mapped pages, with strict TLB invalidation on unmap. Equivalent
+ to passing "iommu.passthrough=0 iommu.strict=1" on the command line.
-config IOMMU_DEFAULT_LAZY
- bool "lazy"
+ Untrusted devices always use this mode, with an additional layer of
+ bounce-buffering such that they cannot gain access to any unrelated
+ data within a mapped page.
+
+config IOMMU_DEFAULT_DMA_LAZY
+ bool "Translated - Lazy"
help
- Support lazy mode, where for every IOMMU DMA unmap operation, the
- flush operation of IOTLB and the free operation of IOVA are deferred.
- They are only guaranteed to be done before the related IOVA will be
- reused.
+ Trusted devices use translation to restrict their access to only
+ DMA-mapped pages, but with "lazy" batched TLB invalidation. This
+ mode allows higher performance with some IOMMUs due to reduced TLB
+ flushing, but at the cost of reduced
[PATCH v4 21/24] iommu: Expose DMA domain strictness via sysfs
The sysfs interface for default domain types exists primarily so users can choose the performance/security tradeoff relevant to their own workload. As such, the choice between the policies for DMA domains fits perfectly as an additional point on that scale - downgrading a particular device from a strict default to non-strict may be enough to let it reach the desired level of performance, while still retaining more peace of mind than with a wide-open identity domain. Now that we've abstracted non-strict mode as a distinct type of DMA domain, allow it to be chosen through the user interface as well. Reviewed-by: Lu Baolu Reviewed-by: John Garry Signed-off-by: Robin Murphy --- v3: Summarise the implications in the documentation for completeness --- Documentation/ABI/testing/sysfs-kernel-iommu_groups | 6 +- drivers/iommu/iommu.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups index eae2f1c1e11e..b15af6a5bc08 100644 --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups @@ -42,8 +42,12 @@ Description: /sys/kernel/iommu_groups//type shows the type of default == DMA All the DMA transactions from the device in this group are translated by the iommu. + DMA-FQAs above, but using batched invalidation to lazily + remove translations after use. This may offer reduced + overhead at the cost of reduced memory protection. identity All the DMA transactions from the device in this group - are not translated by the iommu. + are not translated by the iommu. Maximum performance + but zero protection. auto Change to the type the device was booted with. == diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 55ca5bf3cafc..b141161d5bbc 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -3267,6 +3267,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, req_type = IOMMU_DOMAIN_IDENTITY; else if (sysfs_streq(buf, "DMA")) req_type = IOMMU_DOMAIN_DMA; + else if (sysfs_streq(buf, "DMA-FQ")) + req_type = IOMMU_DOMAIN_DMA_FQ; else if (sysfs_streq(buf, "auto")) req_type = 0; else -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 22/24] iommu: Only log strictness for DMA domains
When passthrough is enabled, the default strictness policy becomes
irrelevant, since any subsequent runtime override to a DMA domain type
now embodies an explicit choice of strictness as well. Save on noise by
only logging the default policy when it is meaningfully in effect.
Reviewed-by: John Garry
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
drivers/iommu/iommu.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b141161d5bbc..63face36fc49 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -144,10 +144,11 @@ static int __init iommu_subsys_init(void)
(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
"(set via kernel command line)" : "");
- pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
- iommu_dma_strict ? "strict" : "lazy",
- (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
- "(set via kernel command line)" : "");
+ if (!iommu_default_passthrough())
+ pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
+ iommu_dma_strict ? "strict" : "lazy",
+ (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
+ "(set via kernel command line)" : "");
return 0;
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 20/24] iommu: Express DMA strictness via the domain type
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only end up using flush queues for drivers
which are aware of them and can actually benefit.
- Allows us to handle flush queue init failure by falling back
to strict mode instead of leaving it to possibly blow up later.
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
v3: Remember to update iommu_def_domain_type accordingly from
iommu_set_dma_strict() too
---
drivers/iommu/dma-iommu.c | 10 ++
drivers/iommu/iommu.c | 14 +-
include/linux/iommu.h | 1 -
3 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 207c8febdac9..01fa83229118 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -363,13 +363,15 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
init_iova_domain(iovad, 1UL << order, base_pfn);
- if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
- domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
+ /* If the FQ fails we can simply fall back to strict mode */
+ if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain) {
if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
- iommu_dma_entry_dtor))
+ iommu_dma_entry_dtor)) {
pr_warn("iova flush queue initialization failed\n");
- else
+ domain->type = IOMMU_DOMAIN_DMA;
+ } else {
cookie->fq_domain = domain;
+ }
}
return iova_reserve_iommu_regions(dev, domain);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 982545234cf3..55ca5bf3cafc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
}
}
+ if (!iommu_default_passthrough() && !iommu_dma_strict)
+ iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
+
pr_info("Default domain type: %s %s\n",
iommu_domain_type_str(iommu_def_domain_type),
(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
@@ -355,17 +358,10 @@ early_param("iommu.strict", iommu_dma_setup);
void iommu_set_dma_strict(void)
{
iommu_dma_strict = true;
+ if (iommu_def_domain_type == IOMMU_DOMAIN_DMA_FQ)
+ iommu_def_domain_type = IOMMU_DOMAIN_DMA;
}
-bool iommu_get_dma_strict(struct iommu_domain *domain)
-{
- /* only allow lazy flushing for DMA domains */
- if (domain->type == IOMMU_DOMAIN_DMA)
- return iommu_dma_strict;
- return true;
-}
-EXPORT_SYMBOL_GPL(iommu_get_dma_strict);
-
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5629ae42951f..923a8d1c5e39 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -504,7 +504,6 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain,
unsigned long quirks);
void iommu_set_dma_strict(void);
-bool iommu_get_dma_strict(struct iommu_domain *domain);
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
unsigned long iova, int flags);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 19/24] iommu/vt-d: Prepare for multiple DMA domain types
In preparation for the strict vs. non-strict decision for DMA domains
to be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops reset/setup can simply be made unconditional, since iommu-dma
already knows only to touch DMA domains.
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
drivers/iommu/intel/iommu.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 7e168634c433..8fc46c9d6b96 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -582,7 +582,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain
*domain)
int iommu_id;
/* si_domain and vm domain should not get here. */
- if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
+ if (WARN_ON(!iommu_is_dma_domain(>domain)))
return NULL;
for_each_domain_iommu(iommu_id, domain)
@@ -1034,7 +1034,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain
*domain,
pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) <<
VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
if (domain_use_first_level(domain)) {
pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
+ if (iommu_is_dma_domain(>domain))
pteval |= DMA_FL_PTE_ACCESS;
}
if (cmpxchg64(>val, 0ULL, pteval))
@@ -2345,7 +2345,7 @@ __domain_mapping(struct dmar_domain *domain, unsigned
long iov_pfn,
if (domain_use_first_level(domain)) {
attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+ if (iommu_is_dma_domain(>domain)) {
attr |= DMA_FL_PTE_ACCESS;
if (prot & DMA_PTE_WRITE)
attr |= DMA_FL_PTE_DIRTY;
@@ -4528,6 +4528,7 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
switch (type) {
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
case IOMMU_DOMAIN_UNMANAGED:
dmar_domain = alloc_domain(0);
if (!dmar_domain) {
@@ -5197,12 +5198,8 @@ static void intel_iommu_release_device(struct device
*dev)
static void intel_iommu_probe_finalize(struct device *dev)
{
- struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
- if (domain && domain->type == IOMMU_DOMAIN_DMA)
- iommu_setup_dma_ops(dev, 0, U64_MAX);
- else
- set_dma_ops(dev, NULL);
+ set_dma_ops(dev, NULL);
+ iommu_setup_dma_ops(dev, 0, U64_MAX);
}
static void intel_iommu_get_resv_regions(struct device *device,
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 18/24] iommu/arm-smmu: Prepare for multiple DMA domain types
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type.
Signed-off-by: Robin Murphy
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
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 d9c93d8d193d..883a99cb10c1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1972,6 +1972,7 @@ static struct iommu_domain
*arm_smmu_domain_alloc(unsigned type)
if (type != IOMMU_DOMAIN_UNMANAGED &&
type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_DMA_FQ &&
type != IOMMU_DOMAIN_IDENTITY)
return NULL;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index a325d4769c17..1d013b1d7bb2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -866,7 +866,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned
type)
struct arm_smmu_domain *smmu_domain;
if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY) {
- if (using_legacy_binding || type != IOMMU_DOMAIN_DMA)
+ if (using_legacy_binding ||
+ (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_DMA_FQ))
return NULL;
}
/*
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 17/24] iommu/amd: Prepare for multiple DMA domain types
The DMA ops reset/setup can simply be unconditional, since
iommu-dma already knows only to touch DMA domains.
Signed-off-by: Robin Murphy
---
drivers/iommu/amd/iommu.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 0fd98d35d73b..02f9b4fffe90 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1707,14 +1707,9 @@ static struct iommu_device
*amd_iommu_probe_device(struct device *dev)
static void amd_iommu_probe_finalize(struct device *dev)
{
- struct iommu_domain *domain;
-
/* Domains are initialized for this device - have a look what we ended
up with */
- domain = iommu_get_domain_for_dev(dev);
- if (domain->type == IOMMU_DOMAIN_DMA)
- iommu_setup_dma_ops(dev, 0, U64_MAX);
- else
- set_dma_ops(dev, NULL);
+ set_dma_ops(dev, NULL);
+ iommu_setup_dma_ops(dev, 0, U64_MAX);
}
static void amd_iommu_release_device(struct device *dev)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 15/24] iommu/io-pgtable: Remove non-strict quirk
IO_PGTABLE_QUIRK_NON_STRICT was never a very comfortable fit, since it's
not a quirk of the pagetable format itself. Now that we have a more
appropriate way to convey non-strict unmaps, though, this last of the
non-quirk quirks can also go, and with the flush queue code also now
enforcing its own ordering we can have a lovely cleanup all round.
Signed-off-by: Robin Murphy
---
v3: New
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 ---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ---
drivers/iommu/io-pgtable-arm-v7s.c | 12 ++--
drivers/iommu/io-pgtable-arm.c | 12 ++--
include/linux/io-pgtable.h | 5 -
5 files changed, 4 insertions(+), 31 deletions(-)
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 4c648da447bf..d9c93d8d193d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2174,9 +2174,6 @@ static int arm_smmu_domain_finalise(struct iommu_domain
*domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
- pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
-
pgtbl_ops = alloc_io_pgtable_ops(fmt, _cfg, smmu_domain);
if (!pgtbl_ops)
return -ENOMEM;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 970d9e4dcd69..a325d4769c17 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -765,9 +765,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain
*domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
- pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
-
if (smmu->impl && smmu->impl->init_context) {
ret = smmu->impl->init_context(smmu_domain, _cfg, dev);
if (ret)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
b/drivers/iommu/io-pgtable-arm-v7s.c
index 5db90d7ce2ec..e84478d39705 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -700,14 +700,7 @@ static size_t __arm_v7s_unmap(struct arm_v7s_io_pgtable
*data,
ARM_V7S_BLOCK_SIZE(lvl + 1));
ptep = iopte_deref(pte[i], lvl, data);
__arm_v7s_free_table(ptep, lvl + 1, data);
- } else if (iop->cfg.quirks &
IO_PGTABLE_QUIRK_NON_STRICT) {
- /*
-* Order the PTE update against queueing the
IOVA, to
-* guarantee that a flush callback from a
different CPU
-* has observed it before the TLBIALL can be
issued.
-*/
- smp_wmb();
- } else {
+ } else if (!gather->queued) {
io_pgtable_tlb_add_page(iop, gather, iova,
blk_size);
}
iova += blk_size;
@@ -791,8 +784,7 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct
io_pgtable_cfg *cfg,
if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NO_PERMS |
- IO_PGTABLE_QUIRK_ARM_MTK_EXT |
- IO_PGTABLE_QUIRK_NON_STRICT))
+ IO_PGTABLE_QUIRK_ARM_MTK_EXT))
return NULL;
/* If ARM_MTK_4GB is enabled, the NO_PERMS is also expected. */
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 053df4048a29..48a5bd8f571d 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -638,14 +638,7 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable
*data,
io_pgtable_tlb_flush_walk(iop, iova + i * size,
size,
ARM_LPAE_GRANULE(data));
__arm_lpae_free_pgtable(data, lvl + 1,
iopte_deref(pte, data));
- } else if (iop->cfg.quirks &
IO_PGTABLE_QUIRK_NON_STRICT) {
- /*
-* Order the PTE update against queueing the
IOVA, to
-* guarantee that a flush callback from a
different CPU
-* has observed it before the TLBIALL can be
issued.
-*/
- smp_wmb();
- } else {
+ } else if (!gather->queued) {
io_pgtable_tlb_add_page(iop, gather, iova + i *
size, size);
}
@@ -825,7 +818,6 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
void
[PATCH v4 16/24] iommu: Introduce explicit type for non-strict DMA domains
Promote the difference between strict and non-strict DMA domains from an
internal detail to a distinct domain feature and type, to pave the road
for exposing it through the sysfs default domain interface.
Reviewed-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
Signed-off-by: Robin Murphy
---
drivers/iommu/dma-iommu.c | 2 +-
drivers/iommu/iommu.c | 8 ++--
include/linux/iommu.h | 11 +++
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d63b30a7dc82..207c8febdac9 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1312,7 +1312,7 @@ void iommu_setup_dma_ops(struct device *dev, u64
dma_base, u64 dma_limit)
* The IOMMU core code allocates the default DMA domain, which the
* underlying IOMMU driver needs to support via the dma-iommu layer.
*/
- if (domain->type == IOMMU_DOMAIN_DMA) {
+ if (iommu_is_dma_domain(domain)) {
if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
goto out_err;
dev->dma_ops = _dma_ops;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fa8109369f74..982545234cf3 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -115,6 +115,7 @@ static const char *iommu_domain_type_str(unsigned int t)
case IOMMU_DOMAIN_UNMANAGED:
return "Unmanaged";
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
return "Translated";
default:
return "Unknown";
@@ -552,6 +553,9 @@ static ssize_t iommu_group_show_type(struct iommu_group
*group,
case IOMMU_DOMAIN_DMA:
type = "DMA\n";
break;
+ case IOMMU_DOMAIN_DMA_FQ:
+ type = "DMA-FQ\n";
+ break;
}
}
mutex_unlock(>mutex);
@@ -765,7 +769,7 @@ static int iommu_create_device_direct_mappings(struct
iommu_group *group,
unsigned long pg_size;
int ret = 0;
- if (!domain || domain->type != IOMMU_DOMAIN_DMA)
+ if (!domain || !iommu_is_dma_domain(domain))
return 0;
BUG_ON(!domain->pgsize_bitmap);
@@ -1947,7 +1951,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct
bus_type *bus,
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
- if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain)) {
+ if (iommu_is_dma_domain(domain) && iommu_get_dma_cookie(domain)) {
iommu_domain_free(domain);
domain = NULL;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f7679f6684b1..5629ae42951f 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -61,6 +61,7 @@ struct iommu_domain_geometry {
#define __IOMMU_DOMAIN_DMA_API (1U << 1) /* Domain for use in DMA-API
implementation */
#define __IOMMU_DOMAIN_PT (1U << 2) /* Domain is identity mapped */
+#define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue*/
/*
* This are the possible domain-types
@@ -73,12 +74,17 @@ struct iommu_domain_geometry {
* IOMMU_DOMAIN_DMA- Internally used for DMA-API implementations.
* This flag allows IOMMU drivers to implement
* certain optimizations for these domains
+ * IOMMU_DOMAIN_DMA_FQ - As above, but definitely using batched TLB
+ * invalidation.
*/
#define IOMMU_DOMAIN_BLOCKED (0U)
#define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT)
#define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
#define IOMMU_DOMAIN_DMA (__IOMMU_DOMAIN_PAGING |\
__IOMMU_DOMAIN_DMA_API)
+#define IOMMU_DOMAIN_DMA_FQ(__IOMMU_DOMAIN_PAGING |\
+__IOMMU_DOMAIN_DMA_API | \
+__IOMMU_DOMAIN_DMA_FQ)
struct iommu_domain {
unsigned type;
@@ -90,6 +96,11 @@ struct iommu_domain {
struct iommu_dma_cookie *iova_cookie;
};
+static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
+{
+ return domain->type & __IOMMU_DOMAIN_DMA_API;
+}
+
enum iommu_cap {
IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
transactions */
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 14/24] iommu: Indicate queued flushes via gather data
Since iommu_iotlb_gather exists to help drivers optimise flushing for a
given unmap request, it is also the logical place to indicate whether
the unmap is strict or not, and thus help them further optimise for
whether to expect a sync or a flush_all subsequently. As part of that,
it also seems fair to make the flush queue code take responsibility for
enforcing the really subtle ordering requirement it brings, so that we
don't need to worry about forgetting that if new drivers want to add
flush queue support, and can consolidate the existing versions.
While we're adding to the kerneldoc, also fill in some info for
@freelist which was overlooked previously.
Signed-off-by: Robin Murphy
---
v3: New
---
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/iova.c | 7 +++
include/linux/iommu.h | 8 +++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index e28396cea6eb..d63b30a7dc82 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -474,6 +474,7 @@ static void __iommu_dma_unmap(struct device *dev,
dma_addr_t dma_addr,
dma_addr -= iova_off;
size = iova_align(iovad, size + iova_off);
iommu_iotlb_gather_init(_gather);
+ iotlb_gather.queued = cookie->fq_domain;
unmapped = iommu_unmap_fast(domain, dma_addr, size, _gather);
WARN_ON(unmapped != size);
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index b6cf5f16123b..2ad73fb2e94e 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -637,6 +637,13 @@ void queue_iova(struct iova_domain *iovad,
unsigned long flags;
unsigned idx;
+ /*
+* Order against the IOMMU driver's pagetable update from unmapping
+* @pte, to guarantee that iova_domain_flush() observes that if called
+* from a different CPU before we release the lock below.
+*/
+ smp_wmb();
+
spin_lock_irqsave(>lock, flags);
/*
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 141779d76035..f7679f6684b1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -161,16 +161,22 @@ enum iommu_dev_features {
* @start: IOVA representing the start of the range to be flushed
* @end: IOVA representing the end of the range to be flushed (inclusive)
* @pgsize: The interval at which to perform the flush
+ * @freelist: Removed pages to free after sync
+ * @queued: Indicates that the flush will be queued
*
* This structure is intended to be updated by multiple calls to the
* ->unmap() function in struct iommu_ops before eventually being passed
- * into ->iotlb_sync().
+ * into ->iotlb_sync(). Drivers can add pages to @freelist to be freed after
+ * ->iotlb_sync() or ->iotlb_flush_all() have cleared all cached references to
+ * them. @queued is set to indicate when ->iotlb_flush_all() will be called
+ * later instead of ->iotlb_sync(), so drivers may optimise accordingly.
*/
struct iommu_iotlb_gather {
unsigned long start;
unsigned long end;
size_t pgsize;
struct page *freelist;
+ boolqueued;
};
/**
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 13/24] iommu/dma: Remove redundant "!dev" checks
iommu_dma_init_domain() is now only called from iommu_setup_dma_ops(),
which has already assumed dev to be non-NULL.
Reviewed-by: John Garry
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
drivers/iommu/dma-iommu.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 10067fbc4309..e28396cea6eb 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -363,7 +363,7 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
init_iova_domain(iovad, 1UL << order, base_pfn);
- if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
+ if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
iommu_dma_entry_dtor))
@@ -372,9 +372,6 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
cookie->fq_domain = domain;
}
- if (!dev)
- return 0;
-
return iova_reserve_iommu_regions(dev, domain);
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 12/24] iommu/dma: Unexport IOVA cookie management
IOVA cookies are now got and put by core code, so we no longer need to
export these to modular drivers. The export for getting MSI cookies
stays, since VFIO can still be a module, but it was already relying on
someone else putting them, so that aspect is unaffected.
Reviewed-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
Signed-off-by: Robin Murphy
---
drivers/iommu/dma-iommu.c | 7 ---
drivers/iommu/iommu.c | 3 +--
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..10067fbc4309 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -98,9 +98,6 @@ static struct iommu_dma_cookie *cookie_alloc(enum
iommu_dma_cookie_type type)
/**
* iommu_get_dma_cookie - Acquire DMA-API resources for a domain
* @domain: IOMMU domain to prepare for DMA-API usage
- *
- * IOMMU drivers should normally call this from their domain_alloc
- * callback when domain->type == IOMMU_DOMAIN_DMA.
*/
int iommu_get_dma_cookie(struct iommu_domain *domain)
{
@@ -113,7 +110,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
return 0;
}
-EXPORT_SYMBOL(iommu_get_dma_cookie);
/**
* iommu_get_msi_cookie - Acquire just MSI remapping resources
@@ -151,8 +147,6 @@ EXPORT_SYMBOL(iommu_get_msi_cookie);
* iommu_put_dma_cookie - Release a domain's DMA mapping resources
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie() or
* iommu_get_msi_cookie()
- *
- * IOMMU drivers should normally call this from their domain_free callback.
*/
void iommu_put_dma_cookie(struct iommu_domain *domain)
{
@@ -172,7 +166,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
kfree(cookie);
domain->iova_cookie = NULL;
}
-EXPORT_SYMBOL(iommu_put_dma_cookie);
/**
* iommu_dma_get_resv_regions - Reserved region driver helper
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b65fcc66ffa4..fa8109369f74 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1947,8 +1947,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct
bus_type *bus,
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
- /* Temporarily avoid -EEXIST while drivers still get their own cookies
*/
- if (type == IOMMU_DOMAIN_DMA && !domain->iova_cookie &&
iommu_get_dma_cookie(domain)) {
+ if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain)) {
iommu_domain_free(domain);
domain = NULL;
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 11/24] iommu/virtio: Drop IOVA cookie management
The core code bakes its own cookies now.
Reviewed-by: Jean-Philippe Brucker
Signed-off-by: Robin Murphy
---
drivers/iommu/virtio-iommu.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 6abdcab7273b..80930ce04a16 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -598,12 +598,6 @@ static struct iommu_domain *viommu_domain_alloc(unsigned
type)
spin_lock_init(>mappings_lock);
vdomain->mappings = RB_ROOT_CACHED;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(>domain)) {
- kfree(vdomain);
- return NULL;
- }
-
return >domain;
}
@@ -643,8 +637,6 @@ static void viommu_domain_free(struct iommu_domain *domain)
{
struct viommu_domain *vdomain = to_viommu_domain(domain);
- iommu_put_dma_cookie(domain);
-
/* Free all remaining mappings (size 2^64) */
viommu_del_mappings(vdomain, 0, 0);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 10/24] iommu/sun50i: Drop IOVA cookie management
The core code bakes its own cookies now. CC: Maxime Ripard Signed-off-by: Robin Murphy --- v3: Also remove unneeded include --- drivers/iommu/sun50i-iommu.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c index 181bb1c3437c..92997021e188 100644 --- a/drivers/iommu/sun50i-iommu.c +++ b/drivers/iommu/sun50i-iommu.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -610,14 +609,10 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type) if (!sun50i_domain) return NULL; - if (type == IOMMU_DOMAIN_DMA && - iommu_get_dma_cookie(_domain->domain)) - goto err_free_domain; - sun50i_domain->dt = (u32 *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, get_order(DT_SIZE)); if (!sun50i_domain->dt) - goto err_put_cookie; + goto err_free_domain; refcount_set(_domain->refcnt, 1); @@ -627,10 +622,6 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type) return _domain->domain; -err_put_cookie: - if (type == IOMMU_DOMAIN_DMA) - iommu_put_dma_cookie(_domain->domain); - err_free_domain: kfree(sun50i_domain); @@ -644,8 +635,6 @@ static void sun50i_iommu_domain_free(struct iommu_domain *domain) free_pages((unsigned long)sun50i_domain->dt, get_order(DT_SIZE)); sun50i_domain->dt = NULL; - iommu_put_dma_cookie(domain); - kfree(sun50i_domain); } -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 09/24] iommu/sprd: Drop IOVA cookie management
The core code bakes its own cookies now.
Acked-by: Chunyan Zhang
Signed-off-by: Robin Murphy
---
v3: Also remove unneeded include
---
drivers/iommu/sprd-iommu.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index 73dfd9946312..27ac818b0354 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -8,7 +8,6 @@
#include
#include
-#include
#include
#include
#include
@@ -144,11 +143,6 @@ static struct iommu_domain
*sprd_iommu_domain_alloc(unsigned int domain_type)
if (!dom)
return NULL;
- if (iommu_get_dma_cookie(>domain)) {
- kfree(dom);
- return NULL;
- }
-
spin_lock_init(>pgtlock);
dom->domain.geometry.aperture_start = 0;
@@ -161,7 +155,6 @@ static void sprd_iommu_domain_free(struct iommu_domain
*domain)
{
struct sprd_iommu_domain *dom = to_sprd_domain(domain);
- iommu_put_dma_cookie(domain);
kfree(dom);
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 08/24] iommu/rockchip: Drop IOVA cookie management
The core code bakes its own cookies now. Tested-by: Heiko Stuebner Acked-by: Heiko Stuebner Signed-off-by: Robin Murphy --- v3: Also remove unneeded include --- drivers/iommu/rockchip-iommu.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 9febfb7f3025..5cb260820eda 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -1074,10 +1073,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) if (!rk_domain) return NULL; - if (type == IOMMU_DOMAIN_DMA && - iommu_get_dma_cookie(_domain->domain)) - goto err_free_domain; - /* * rk32xx iommus use a 2 level pagetable. * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries. @@ -1085,7 +1080,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) */ rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32); if (!rk_domain->dt) - goto err_put_cookie; + goto err_free_domain; rk_domain->dt_dma = dma_map_single(dma_dev, rk_domain->dt, SPAGE_SIZE, DMA_TO_DEVICE); @@ -1106,9 +1101,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) err_free_dt: free_page((unsigned long)rk_domain->dt); -err_put_cookie: - if (type == IOMMU_DOMAIN_DMA) - iommu_put_dma_cookie(_domain->domain); err_free_domain: kfree(rk_domain); @@ -1137,8 +1129,6 @@ static void rk_iommu_domain_free(struct iommu_domain *domain) SPAGE_SIZE, DMA_TO_DEVICE); free_page((unsigned long)rk_domain->dt); - if (domain->type == IOMMU_DOMAIN_DMA) - iommu_put_dma_cookie(_domain->domain); kfree(rk_domain); } -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 07/24] iommu/mtk: Drop IOVA cookie management
The core code bakes its own cookies now.
CC: Yong Wu
Signed-off-by: Robin Murphy
---
v3: Also remove unneeded includes
---
drivers/iommu/mtk_iommu.c| 7 ---
drivers/iommu/mtk_iommu_v1.c | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6f7c69688ce2..185694eb4456 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -441,17 +440,11 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
if (!dom)
return NULL;
- if (iommu_get_dma_cookie(>domain)) {
- kfree(dom);
- return NULL;
- }
-
return >domain;
}
static void mtk_iommu_domain_free(struct iommu_domain *domain)
{
- iommu_put_dma_cookie(domain);
kfree(to_mtk_domain(domain));
}
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 778e66f5f1aa..be22fcf988ce 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -13,7 +13,6 @@
#include
#include
#include
-#include
#include
#include
#include
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 06/24] iommu/ipmmu-vmsa: Drop IOVA cookie management
The core code bakes its own cookies now.
Reviewed-by: Yoshihiro Shimoda
Tested-by: Yoshihiro Shimoda
Signed-off-by: Robin Murphy
---
v3: Also remove unneeded include
---
drivers/iommu/ipmmu-vmsa.c | 28
1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 51ea6f00db2f..d38ff29a76e8 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -8,7 +8,6 @@
#include
#include
-#include
#include
#include
#include
@@ -564,10 +563,13 @@ static irqreturn_t ipmmu_irq(int irq, void *dev)
* IOMMU Operations
*/
-static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
{
struct ipmmu_vmsa_domain *domain;
+ if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+ return NULL;
+
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
if (!domain)
return NULL;
@@ -577,27 +579,6 @@ static struct iommu_domain *__ipmmu_domain_alloc(unsigned
type)
return >io_domain;
}
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
- struct iommu_domain *io_domain = NULL;
-
- switch (type) {
- case IOMMU_DOMAIN_UNMANAGED:
- io_domain = __ipmmu_domain_alloc(type);
- break;
-
- case IOMMU_DOMAIN_DMA:
- io_domain = __ipmmu_domain_alloc(type);
- if (io_domain && iommu_get_dma_cookie(io_domain)) {
- kfree(io_domain);
- io_domain = NULL;
- }
- break;
- }
-
- return io_domain;
-}
-
static void ipmmu_domain_free(struct iommu_domain *io_domain)
{
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -606,7 +587,6 @@ static void ipmmu_domain_free(struct iommu_domain
*io_domain)
* Free the domain resources. We assume that all devices have already
* been detached.
*/
- iommu_put_dma_cookie(io_domain);
ipmmu_domain_destroy_context(domain);
free_io_pgtable_ops(domain->iop);
kfree(domain);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 05/24] iommu/exynos: Drop IOVA cookie management
The core code bakes its own cookies now.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Robin Murphy
---
v3: Also remove unneeded include
---
drivers/iommu/exynos-iommu.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index d0fbf1d10e18..939ffa768986 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
typedef u32 sysmmu_iova_t;
typedef u32 sysmmu_pte_t;
@@ -735,20 +734,16 @@ static struct iommu_domain
*exynos_iommu_domain_alloc(unsigned type)
/* Check if correct PTE offsets are initialized */
BUG_ON(PG_ENT_SHIFT < 0 || !dma_dev);
+ if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
+ return NULL;
+
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
if (!domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA) {
- if (iommu_get_dma_cookie(>domain) != 0)
- goto err_pgtable;
- } else if (type != IOMMU_DOMAIN_UNMANAGED) {
- goto err_pgtable;
- }
-
domain->pgtable = (sysmmu_pte_t *)__get_free_pages(GFP_KERNEL, 2);
if (!domain->pgtable)
- goto err_dma_cookie;
+ goto err_pgtable;
domain->lv2entcnt = (short *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
1);
if (!domain->lv2entcnt)
@@ -779,9 +774,6 @@ static struct iommu_domain
*exynos_iommu_domain_alloc(unsigned type)
free_pages((unsigned long)domain->lv2entcnt, 1);
err_counter:
free_pages((unsigned long)domain->pgtable, 2);
-err_dma_cookie:
- if (type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(>domain);
err_pgtable:
kfree(domain);
return NULL;
@@ -809,9 +801,6 @@ static void exynos_iommu_domain_free(struct iommu_domain
*iommu_domain)
spin_unlock_irqrestore(>lock, flags);
- if (iommu_domain->type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(iommu_domain);
-
dma_unmap_single(dma_dev, virt_to_phys(domain->pgtable), LV1TABLE_SIZE,
DMA_TO_DEVICE);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 04/24] iommu/vt-d: Drop IOVA cookie management
The core code bakes its own cookies now.
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
drivers/iommu/intel/iommu.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index c12cc955389a..7e168634c433 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1979,10 +1979,6 @@ static void domain_exit(struct dmar_domain *domain)
/* Remove associated devices and clear attached or cached domains */
domain_remove_dev_info(domain);
- /* destroy iovas */
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(>domain);
-
if (domain->pgd) {
struct page *freelist;
@@ -4544,10 +4540,6 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
return NULL;
}
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(_domain->domain))
- return NULL;
-
domain = _domain->domain;
domain->geometry.aperture_start = 0;
domain->geometry.aperture_end =
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 03/24] iommu/arm-smmu: Drop IOVA cookie management
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 ---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 15 ---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 9 -
3 files changed, 4 insertions(+), 27 deletions(-)
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 6346f21726f4..4c648da447bf 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1984,12 +1984,6 @@ static struct iommu_domain
*arm_smmu_domain_alloc(unsigned type)
if (!smmu_domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(_domain->domain)) {
- kfree(smmu_domain);
- return NULL;
- }
-
mutex_init(_domain->init_mutex);
INIT_LIST_HEAD(_domain->devices);
spin_lock_init(_domain->devices_lock);
@@ -2021,7 +2015,6 @@ static void arm_smmu_domain_free(struct iommu_domain
*domain)
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
- iommu_put_dma_cookie(domain);
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
/* Free the CD and ASID, if we allocated them */
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index ac21170fa208..970d9e4dcd69 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -868,10 +868,10 @@ static struct iommu_domain
*arm_smmu_domain_alloc(unsigned type)
{
struct arm_smmu_domain *smmu_domain;
- if (type != IOMMU_DOMAIN_UNMANAGED &&
- type != IOMMU_DOMAIN_DMA &&
- type != IOMMU_DOMAIN_IDENTITY)
- return NULL;
+ if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY) {
+ if (using_legacy_binding || type != IOMMU_DOMAIN_DMA)
+ return NULL;
+ }
/*
* Allocate the domain and initialise some of its data structures.
* We can't really do anything meaningful until we've added a
@@ -881,12 +881,6 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned
type)
if (!smmu_domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA && (using_legacy_binding ||
- iommu_get_dma_cookie(_domain->domain))) {
- kfree(smmu_domain);
- return NULL;
- }
-
mutex_init(_domain->init_mutex);
spin_lock_init(_domain->cb_lock);
@@ -901,7 +895,6 @@ static void arm_smmu_domain_free(struct iommu_domain
*domain)
* Free the domain resources. We assume that all devices have
* already been detached.
*/
- iommu_put_dma_cookie(domain);
arm_smmu_destroy_domain_context(domain);
kfree(smmu_domain);
}
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 021cf8f65ffc..b91874cb6cf3 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -10,7 +10,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -335,12 +334,6 @@ static struct iommu_domain
*qcom_iommu_domain_alloc(unsigned type)
if (!qcom_domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(_domain->domain)) {
- kfree(qcom_domain);
- return NULL;
- }
-
mutex_init(_domain->init_mutex);
spin_lock_init(_domain->pgtbl_lock);
@@ -351,8 +344,6 @@ static void qcom_iommu_domain_free(struct iommu_domain
*domain)
{
struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
- iommu_put_dma_cookie(domain);
-
if (qcom_domain->iommu) {
/*
* NOTE: unmap can be called after client device is powered
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 02/24] iommu/amd: Drop IOVA cookie management
The core code bakes its own cookies now. Signed-off-by: Robin Murphy --- drivers/iommu/amd/iommu.c | 12 1 file changed, 12 deletions(-) diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index 52fe2326042a..0fd98d35d73b 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -1918,16 +1918,7 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned type) domain->domain.geometry.aperture_end = ~0ULL; domain->domain.geometry.force_aperture = true; - if (type == IOMMU_DOMAIN_DMA && - iommu_get_dma_cookie(>domain) == -ENOMEM) - goto free_domain; - return >domain; - -free_domain: - protection_domain_free(domain); - - return NULL; } static void amd_iommu_domain_free(struct iommu_domain *dom) @@ -1944,9 +1935,6 @@ static void amd_iommu_domain_free(struct iommu_domain *dom) if (!dom) return; - if (dom->type == IOMMU_DOMAIN_DMA) - iommu_put_dma_cookie(>domain); - if (domain->flags & PD_IOMMUV2_MASK) free_gcr3_table(domain); -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 01/24] iommu: Pull IOVA cookie management into the core
Now that everyone has converged on iommu-dma for IOMMU_DOMAIN_DMA
support, we can abandon the notion of drivers being responsible for the
cookie type, and consolidate all the management into the core code.
CC: Yong Wu
CC: Chunyan Zhang
CC: Maxime Ripard
Tested-by: Heiko Stuebner
Tested-by: Marek Szyprowski
Tested-by: Yoshihiro Shimoda
Reviewed-by: Jean-Philippe Brucker
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
v3: Use a simpler temporary check instead of trying to be clever with
the error code
---
drivers/iommu/iommu.c | 7 +++
include/linux/iommu.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2cda9950bd5..b65fcc66ffa4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt)"iommu: " fmt
#include
+#include
#include
#include
#include
@@ -1946,6 +1947,11 @@ static struct iommu_domain *__iommu_domain_alloc(struct
bus_type *bus,
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
+ /* Temporarily avoid -EEXIST while drivers still get their own cookies
*/
+ if (type == IOMMU_DOMAIN_DMA && !domain->iova_cookie &&
iommu_get_dma_cookie(domain)) {
+ iommu_domain_free(domain);
+ domain = NULL;
+ }
return domain;
}
@@ -1957,6 +1963,7 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
void iommu_domain_free(struct iommu_domain *domain)
{
+ iommu_put_dma_cookie(domain);
domain->ops->domain_free(domain);
}
EXPORT_SYMBOL_GPL(iommu_domain_free);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 4997c78e2670..141779d76035 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -40,6 +40,7 @@ struct iommu_domain;
struct notifier_block;
struct iommu_sva;
struct iommu_fault_event;
+struct iommu_dma_cookie;
/* iommu fault flags */
#define IOMMU_FAULT_READ 0x0
@@ -86,7 +87,7 @@ struct iommu_domain {
iommu_fault_handler_t handler;
void *handler_token;
struct iommu_domain_geometry geometry;
- void *iova_cookie;
+ struct iommu_dma_cookie *iova_cookie;
};
enum iommu_cap {
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 00/24] iommu: Refactor DMA domain strictness
v1: https://lore.kernel.org/linux-iommu/cover.1626888444.git.robin.mur...@arm.com/ v2: https://lore.kernel.org/linux-iommu/cover.1627468308.git.robin.mur...@arm.com/ v3: https://lore.kernel.org/linux-iommu/cover.1628094600.git.robin.mur...@arm.com/ Hi all, I'm hoping this is good to go now. Changes from v3 are minimal, just rolling back patch #2 to the non-broken version, and tidying up the final patch as documented there. Cheers, Robin. CC: Marek Szyprowski CC: Yoshihiro Shimoda CC: Geert Uytterhoeven CC: Yong Wu CC: Heiko Stuebner CC: Chunyan Zhang CC: Maxime Ripard CC: Jean-Philippe Brucker Robin Murphy (24): iommu: Pull IOVA cookie management into the core iommu/amd: Drop IOVA cookie management iommu/arm-smmu: Drop IOVA cookie management iommu/vt-d: Drop IOVA cookie management iommu/exynos: Drop IOVA cookie management iommu/ipmmu-vmsa: Drop IOVA cookie management iommu/mtk: Drop IOVA cookie management iommu/rockchip: Drop IOVA cookie management iommu/sprd: Drop IOVA cookie management iommu/sun50i: Drop IOVA cookie management iommu/virtio: Drop IOVA cookie management iommu/dma: Unexport IOVA cookie management iommu/dma: Remove redundant "!dev" checks iommu: Indicate queued flushes via gather data iommu/io-pgtable: Remove non-strict quirk iommu: Introduce explicit type for non-strict DMA domains iommu/amd: Prepare for multiple DMA domain types iommu/arm-smmu: Prepare for multiple DMA domain types iommu/vt-d: Prepare for multiple DMA domain types iommu: Express DMA strictness via the domain type iommu: Expose DMA domain strictness via sysfs iommu: Only log strictness for DMA domains iommu: Merge strictness and domain type configs iommu: Allow enabling non-strict mode dynamically .../ABI/testing/sysfs-kernel-iommu_groups | 6 +- .../admin-guide/kernel-parameters.txt | 8 +- drivers/iommu/Kconfig | 80 +-- drivers/iommu/amd/iommu.c | 21 + drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 +-- drivers/iommu/arm/arm-smmu/arm-smmu.c | 19 ++--- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 9 --- drivers/iommu/dma-iommu.c | 58 -- drivers/iommu/exynos-iommu.c | 19 + drivers/iommu/intel/iommu.c | 23 ++ drivers/iommu/io-pgtable-arm-v7s.c| 12 +-- drivers/iommu/io-pgtable-arm.c| 12 +-- drivers/iommu/iommu.c | 56 - drivers/iommu/iova.c | 14 +++- drivers/iommu/ipmmu-vmsa.c| 28 +-- drivers/iommu/mtk_iommu.c | 7 -- drivers/iommu/mtk_iommu_v1.c | 1 - drivers/iommu/rockchip-iommu.c| 12 +-- drivers/iommu/sprd-iommu.c| 7 -- drivers/iommu/sun50i-iommu.c | 13 +-- drivers/iommu/virtio-iommu.c | 8 -- include/linux/dma-iommu.h | 6 ++ include/linux/io-pgtable.h| 5 -- include/linux/iommu.h | 23 +- 24 files changed, 184 insertions(+), 274 deletions(-) -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()
On Tue, Aug 10, 2021 at 02:48:54PM -0500, Tom Lendacky wrote:
> On 8/10/21 1:45 PM, Kuppuswamy, Sathyanarayanan wrote:
> >
> >
> > On 7/27/21 3:26 PM, Tom Lendacky wrote:
> >> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> >> index de01903c3735..cafed6456d45 100644
> >> --- a/arch/x86/kernel/head64.c
> >> +++ b/arch/x86/kernel/head64.c
> >> @@ -19,7 +19,7 @@
> >> #include
> >> #include
> >> #include
> >> -#include
> >> +#include
> >> #include
> >> #include
> >> @@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long
> >> physaddr,
> >> * there is no need to zero it after changing the memory encryption
> >> * attribute.
> >> */
> >> - if (mem_encrypt_active()) {
> >> + if (prot_guest_has(PATTR_MEM_ENCRYPT)) {
> >> vaddr = (unsigned long)__start_bss_decrypted;
> >> vaddr_end = (unsigned long)__end_bss_decrypted;
> >
> >
> > Since this change is specific to AMD, can you replace PATTR_MEM_ENCRYPT with
> > prot_guest_has(PATTR_SME) || prot_guest_has(PATTR_SEV). It is not used in
> > TDX.
>
> This is a direct replacement for now.
With current implementation of prot_guest_has() for TDX it breaks boot for
me.
Looking at code agains, now I *think* the reason is accessing a global
variable from __startup_64() inside TDX version of prot_guest_has().
__startup_64() is special. If you access any global variable you need to
use fixup_pointer(). See comment before __startup_64().
I'm not sure how you get away with accessing sme_me_mask directly from
there. Any clues? Maybe just a luck and complier generates code just right
for your case, I donno.
A separate point is that TDX version of prot_guest_has() relies on
cpu_feature_enabled() which is not ready at this point.
I think __bss_decrypted fixup has to be done if sme_me_mask is non-zero.
Or just do it uncoditionally because it's NOP for sme_me_mask == 0.
> I think the change you're requesting
> should be done as part of the TDX support patches so it's clear why it is
> being changed.
>
> But, wouldn't TDX still need to do something with this shared/unencrypted
> area, though? Or since it is shared, there's actually nothing you need to
> do (the bss decrpyted section exists even if CONFIG_AMD_MEM_ENCRYPT is not
> configured)?
AFAICS, only kvmclock uses __bss_decrypted. We don't enable kvmclock in
TDX at the moment. It may change in the future.
--
Kirill A. Shutemov
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/4] Prepare for ECMDQ support
RFC --> v1 1. Resend the patches for ECMDQ preparation and remove the patches for ECMDQ implementation. 2. Patch 2 is modified. Other patches remain unchanged. 1) Add static helper __arm_smmu_cmdq_issue_cmd(), and make arm_smmu_cmdq_issue_cmd() and arm_smmu_cmdq_issue_cmd_with_sync() implement based on it. 2) Remove unused arm_smmu_cmdq_issue_sync(). RFC: https://www.spinics.net/lists/arm-kernel/msg904879.html Zhen Lei (4): iommu/arm-smmu-v3: Use command queue batching helpers to improve performance iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync() iommu/arm-smmu-v3: Add and use static helper function arm_smmu_get_cmdq() iommu/arm-smmu-v3: Extract reusable function __arm_smmu_cmdq_skip_err() drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 71 - 1 file changed, 42 insertions(+), 29 deletions(-) -- 2.26.0.106.g9fadedd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/4] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_get_cmdq()
One SMMU has only one normal CMDQ. Therefore, this CMDQ is used regardless
of the core on which the command is inserted. It can be referenced
directly through "smmu->cmdq". However, one SMMU has multiple ECMDQs, and
the ECMDQ used by the core on which the command insertion is executed may
be different. So the helper function arm_smmu_get_cmdq() is added, which
returns the CMDQ/ECMDQ that the current core should use. Currently, the
code that supports ECMDQ is not added. just simply returns ">cmdq".
Many subfunctions of arm_smmu_cmdq_issue_cmdlist() use ">cmdq" or
">cmdq.q" directly. To support ECMDQ, they need to call the newly
added function arm_smmu_get_cmdq() instead.
Note that normal CMDQ is still required until ECMDQ is available.
Signed-off-by: Zhen Lei
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 -
1 file changed, 13 insertions(+), 9 deletions(-)
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 282f95659580267..be2df5ad2eb51b8 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -335,10 +335,14 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct
arm_smmu_cmdq_ent *ent)
return 0;
}
+static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
+{
+ return >cmdq;
+}
+
static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device
*smmu,
-u32 prod)
+struct arm_smmu_queue *q, u32 prod)
{
- struct arm_smmu_queue *q = >cmdq.q;
struct arm_smmu_cmdq_ent ent = {
.opcode = CMDQ_OP_CMD_SYNC,
};
@@ -579,7 +583,7 @@ static int arm_smmu_cmdq_poll_until_not_full(struct
arm_smmu_device *smmu,
{
unsigned long flags;
struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = >cmdq;
+ struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
int ret = 0;
/*
@@ -595,7 +599,7 @@ static int arm_smmu_cmdq_poll_until_not_full(struct
arm_smmu_device *smmu,
queue_poll_init(smmu, );
do {
- llq->val = READ_ONCE(smmu->cmdq.q.llq.val);
+ llq->val = READ_ONCE(cmdq->q.llq.val);
if (!queue_full(llq))
break;
@@ -614,7 +618,7 @@ static int __arm_smmu_cmdq_poll_until_msi(struct
arm_smmu_device *smmu,
{
int ret = 0;
struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = >cmdq;
+ struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
u32 *cmd = (u32 *)(Q_ENT(>q, llq->prod));
queue_poll_init(smmu, );
@@ -637,12 +641,12 @@ static int __arm_smmu_cmdq_poll_until_consumed(struct
arm_smmu_device *smmu,
struct arm_smmu_ll_queue *llq)
{
struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = >cmdq;
+ struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
u32 prod = llq->prod;
int ret = 0;
queue_poll_init(smmu, );
- llq->val = READ_ONCE(smmu->cmdq.q.llq.val);
+ llq->val = READ_ONCE(cmdq->q.llq.val);
do {
if (queue_consumed(llq, prod))
break;
@@ -732,7 +736,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct
arm_smmu_device *smmu,
u32 prod;
unsigned long flags;
bool owner;
- struct arm_smmu_cmdq *cmdq = >cmdq;
+ struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
struct arm_smmu_ll_queue llq = {
.max_n_shift = cmdq->q.llq.max_n_shift,
}, head = llq;
@@ -772,7 +776,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct
arm_smmu_device *smmu,
arm_smmu_cmdq_write_entries(cmdq, cmds, llq.prod, n);
if (sync) {
prod = queue_inc_prod_n(, n);
- arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, prod);
+ arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, >q, prod);
queue_write(Q_ENT(>q, prod), cmd_sync, CMDQ_ENT_DWORDS);
/*
--
2.26.0.106.g9fadedd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/4] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync()
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.
Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().
Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert
the 'cmd+sync' commands at a time.
Signed-off-by: Zhen Lei
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 35 +++--
1 file changed, 19 insertions(+), 16 deletions(-)
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 c81cd929047f573..282f95659580267 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -845,8 +845,9 @@ static int arm_smmu_cmdq_issue_cmdlist(struct
arm_smmu_device *smmu,
return ret;
}
-static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
- struct arm_smmu_cmdq_ent *ent)
+static int __arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
+struct arm_smmu_cmdq_ent *ent,
+bool sync)
{
u64 cmd[CMDQ_ENT_DWORDS];
@@ -856,12 +857,19 @@ static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device
*smmu,
return -EINVAL;
}
- return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false);
+ return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, sync);
}
-static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
+static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq_ent *ent)
{
- return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true);
+ return __arm_smmu_cmdq_issue_cmd(smmu, ent, false);
+}
+
+static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
+struct arm_smmu_cmdq_ent *ent)
+{
+ return __arm_smmu_cmdq_issue_cmd(smmu, ent, true);
}
static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
@@ -929,8 +937,7 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu,
u16 asid)
.tlbi.asid = asid,
};
- arm_smmu_cmdq_issue_cmd(smmu, );
- arm_smmu_cmdq_issue_sync(smmu);
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
}
static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
@@ -1211,8 +1218,7 @@ static void arm_smmu_sync_ste_for_sid(struct
arm_smmu_device *smmu, u32 sid)
},
};
- arm_smmu_cmdq_issue_cmd(smmu, );
- arm_smmu_cmdq_issue_sync(smmu);
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
}
static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
@@ -1824,8 +1830,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
} else {
cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
- arm_smmu_cmdq_issue_cmd(smmu, );
- arm_smmu_cmdq_issue_sync(smmu);
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
}
arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
}
@@ -3339,18 +3344,16 @@ static int arm_smmu_device_reset(struct arm_smmu_device
*smmu, bool bypass)
/* Invalidate any cached configuration */
cmd.opcode = CMDQ_OP_CFGI_ALL;
- arm_smmu_cmdq_issue_cmd(smmu, );
- arm_smmu_cmdq_issue_sync(smmu);
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
/* Invalidate any stale TLB entries */
if (smmu->features & ARM_SMMU_FEAT_HYP) {
cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;
- arm_smmu_cmdq_issue_cmd(smmu, );
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
}
cmd.opcode = CMDQ_OP_TLBI_NSNH_ALL;
- arm_smmu_cmdq_issue_cmd(smmu, );
- arm_smmu_cmdq_issue_sync(smmu);
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu, );
/* Event queue */
writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
--
2.26.0.106.g9fadedd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/4] iommu/arm-smmu-v3: Use command queue batching helpers to improve performance
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.
Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().
Therefore, use command queue batching helpers to insert multiple commands
at a time.
Signed-off-by: Zhen Lei
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
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 235f9bdaeaf223b..c81cd929047f573 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1747,15 +1747,16 @@ static int arm_smmu_atc_inv_master(struct
arm_smmu_master *master)
{
int i;
struct arm_smmu_cmdq_ent cmd;
+ struct arm_smmu_cmdq_batch cmds = {};
arm_smmu_atc_inv_to_cmd(0, 0, 0, );
for (i = 0; i < master->num_streams; i++) {
cmd.atc.sid = master->streams[i].id;
- arm_smmu_cmdq_issue_cmd(master->smmu, );
+ arm_smmu_cmdq_batch_add(master->smmu, , );
}
- return arm_smmu_cmdq_issue_sync(master->smmu);
+ return arm_smmu_cmdq_batch_submit(master->smmu, );
}
int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
--
2.26.0.106.g9fadedd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/4] iommu/arm-smmu-v3: Extract reusable function __arm_smmu_cmdq_skip_err()
When SMMU_GERROR.CMDQP_ERR is different to SMMU_GERRORN.CMDQP_ERR, it
indicates that one or more errors have been encountered on a command queue
control page interface. We need to traverse all ECMDQs in that control
page to find all errors. For each ECMDQ error handling, it is much the
same as the CMDQ error handling. This common processing part is extracted
as a new function __arm_smmu_cmdq_skip_err().
Signed-off-by: Zhen Lei
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
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 be2df5ad2eb51b8..597cc0ff5ef40f0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -359,7 +359,8 @@ static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct
arm_smmu_device *smmu,
arm_smmu_cmdq_build_cmd(cmd, );
}
-static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
+static void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
+struct arm_smmu_queue *q)
{
static const char * const cerror_str[] = {
[CMDQ_ERR_CERROR_NONE_IDX] = "No error",
@@ -370,7 +371,6 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device
*smmu)
int i;
u64 cmd[CMDQ_ENT_DWORDS];
- struct arm_smmu_queue *q = >cmdq.q;
u32 cons = readl_relaxed(q->cons_reg);
u32 idx = FIELD_GET(CMDQ_CONS_ERR, cons);
struct arm_smmu_cmdq_ent cmd_sync = {
@@ -417,6 +417,11 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device
*smmu)
queue_write(Q_ENT(q, cons), cmd, q->ent_dwords);
}
+static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
+{
+ __arm_smmu_cmdq_skip_err(smmu, >cmdq.q);
+}
+
/*
* Command queue locking.
* This is a form of bastardised rwlock with the following major changes:
--
2.26.0.106.g9fadedd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 2/8] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync()
Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert
the 'cmd+sync' commands at a time.
Signed-off-by: Zhen Lei
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
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 2433d3c29b49ff2..a5361153ca1d6a4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -858,11 +858,25 @@ static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device
*smmu,
return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false);
}
-static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
+static int __maybe_unused arm_smmu_cmdq_issue_sync(struct arm_smmu_device
*smmu)
{
return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true);
}
+static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
+struct arm_smmu_cmdq_ent *ent)
+{
+ u64 cmd[CMDQ_ENT_DWORDS];
+
+ if (arm_smmu_cmdq_build_cmd(cmd, ent)) {
+ dev_warn(smmu->dev, "ignoring unknown CMDQ opcode 0x%x\n",
+ent->opcode);
+ return -EINVAL;
Are any of the errors returned from the "issue command" functions actually
consumed? I couldn't see it on mainline code from a brief browse.
I don't think so.
I don't think so either.
Can we actually propagate them?
There does appear to be some places, here's one I found:
arm_smmu_page_response() -> arm_smmu_cmdq_issue_cmd(), and
arm_smmu_page_response is set to arm_smmu_ops.page_response, which
returns an int
Thanks,
John
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: APPLE_DART should depend on ARCH_APPLE
Good catch, thanks!
Acked-by: Sven Peter
Sven
On Tue, Aug 10, 2021, at 15:47, Geert Uytterhoeven wrote:
> The Apple DART (Device Address Resolution Table) IOMMU is only present
> on Apple ARM SoCs like the M1. Hence add a dependency on ARCH_APPLE, to
> prevent asking the user about this driver when configuring a kernel
> without support for the Apple Silicon SoC family.
>
> Fixes: 05ce9d20d699b093 ("iommu/dart: Add DART iommu driver")
> Signed-off-by: Geert Uytterhoeven
> ---
> drivers/iommu/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index dfe81da483e9e073..e908b8222e4ed679 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -292,7 +292,7 @@ config SPAPR_TCE_IOMMU
>
> config APPLE_DART
> tristate "Apple DART IOMMU Support"
> - depends on ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)
> + depends on ARCH_APPLE || (COMPILE_TEST && !GENERIC_ATOMIC64)
> select IOMMU_API
> select IOMMU_IO_PGTABLE_LPAE
> default ARCH_APPLE
> --
> 2.25.1
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv4] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
On 2021-08-11 16:00, Will Deacon wrote:
On Wed, Aug 11, 2021 at 11:37:25AM +0530, Sai Prakash Ranjan wrote:
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f7da8953afbe..3904b598e0f9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -327,9 +327,16 @@ static void arm_smmu_tlb_inv_range_s2(unsigned
long iova, size_t size,
static void arm_smmu_tlb_inv_walk_s1(unsigned long iova, size_t size,
size_t granule, void *cookie)
{
- arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
- ARM_SMMU_CB_S1_TLBIVA);
- arm_smmu_tlb_sync_context(cookie);
+ struct arm_smmu_domain *smmu_domain = cookie;
+ struct arm_smmu_cfg *cfg = _domain->cfg;
+
+ if (cfg->flush_walk_prefer_tlbiasid) {
+ arm_smmu_tlb_inv_context_s1(cookie);
Hmm, this introduces an unconditional wmb() if tlbiasid is preferred. I
think that should be predicated on ARM_SMMU_FEAT_COHERENT_WALK like it
is
for the by-VA ops. Worth doing as a separate patch.
Ok I will keep this as-is for now then.
+ } else {
+ arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
+ ARM_SMMU_CB_S1_TLBIVA);
+ arm_smmu_tlb_sync_context(cookie);
+ }
}
static void arm_smmu_tlb_add_page_s1(struct iommu_iotlb_gather
*gather,
@@ -765,8 +772,10 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
+ if (!iommu_get_dma_strict(domain)) {
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ cfg->flush_walk_prefer_tlbiasid = true;
This is going to interact badly with Robin's series to allow dynamic
transition to non-strict mode, as we don't have a mechanism to switch
over to the by-ASID behaviour. Yes, it should _work_, but it's ugly
having
different TLBI behaviour just because of the how the domain became
non-strict.
Robin -- I think this originated from your idea at [1]. Any idea how to
make
it work with your other series, or shall we drop this part for now and
leave
the TLB invalidation behaviour the same for now?
Will
[1]
https://lore.kernel.org/r/da62ff1c-9b49-34d3-69a1-1a674e4a3...@arm.com
Right, I think we can drop this non-strict change for now because it
also makes
it a pain to backport it to 5.4/5.10 kernels because of large number of
changes
in dma apis in recent kernels. I will let you and Robin decide if it's
ok to
drop this change and introduce it later with a different patch.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 2/8] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync()
On Wed, Aug 11, 2021 at 11:31:08AM +0100, John Garry wrote:
> > > > > Obviously, inserting as many commands at a time as possible can
> > > > > reduce the
> > > > > number of times the mutex contention participates, thereby improving
> > > > > the
> > > > > overall performance. At least it reduces the number of calls to
> > > > > function
> > > > > arm_smmu_cmdq_issue_cmdlist().
> > > > >
> > > > > Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to
> > > > > insert
> > > > > the 'cmd+sync' commands at a time.
> > > > >
> > > > > Signed-off-by: Zhen Lei
> > > > > ---
> > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33
> > > > > +
> > > > > 1 file changed, 21 insertions(+), 12 deletions(-)
> > > > >
> > > > > 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 2433d3c29b49ff2..a5361153ca1d6a4 100644
> > > > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > > @@ -858,11 +858,25 @@ static int arm_smmu_cmdq_issue_cmd(struct
> > > > > arm_smmu_device *smmu,
> > > > > return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false);
> > > > > }
> > > > > -static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
> > > > > +static int __maybe_unused arm_smmu_cmdq_issue_sync(struct
> > > > > arm_smmu_device *smmu)
> > > > > {
> > > > > return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true);
> > > > > }
> > > > > +static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device
> > > > > *smmu,
> > > > > + struct arm_smmu_cmdq_ent
> > > > > *ent)
> > > > > +{
> > > > > + u64 cmd[CMDQ_ENT_DWORDS];
> > > > > +
> > > > > + if (arm_smmu_cmdq_build_cmd(cmd, ent)) {
> > > > > + dev_warn(smmu->dev, "ignoring unknown CMDQ opcode
> > > > > 0x%x\n",
> > > > > + ent->opcode);
> > > > > + return -EINVAL;
>
> Are any of the errors returned from the "issue command" functions actually
> consumed? I couldn't see it on mainline code from a brief browse.
I don't think so. Can we actually propagate them?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 2/8] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync()
Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().
Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert
the 'cmd+sync' commands at a time.
Signed-off-by: Zhen Lei
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
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 2433d3c29b49ff2..a5361153ca1d6a4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -858,11 +858,25 @@ static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device
*smmu,
return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false);
}
-static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
+static int __maybe_unused arm_smmu_cmdq_issue_sync(struct arm_smmu_device
*smmu)
{
return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true);
}
+static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
+struct arm_smmu_cmdq_ent *ent)
+{
+ u64 cmd[CMDQ_ENT_DWORDS];
+
+ if (arm_smmu_cmdq_build_cmd(cmd, ent)) {
+ dev_warn(smmu->dev, "ignoring unknown CMDQ opcode 0x%x\n",
+ent->opcode);
+ return -EINVAL;
Are any of the errors returned from the "issue command" functions
actually consumed? I couldn't see it on mainline code from a brief browse.
+ }
+
+ return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, true);
Thanks,
John
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv4] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
On Wed, Aug 11, 2021 at 11:37:25AM +0530, Sai Prakash Ranjan wrote:
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index f7da8953afbe..3904b598e0f9 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -327,9 +327,16 @@ static void arm_smmu_tlb_inv_range_s2(unsigned long
> iova, size_t size,
> static void arm_smmu_tlb_inv_walk_s1(unsigned long iova, size_t size,
>size_t granule, void *cookie)
> {
> - arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
> - ARM_SMMU_CB_S1_TLBIVA);
> - arm_smmu_tlb_sync_context(cookie);
> + struct arm_smmu_domain *smmu_domain = cookie;
> + struct arm_smmu_cfg *cfg = _domain->cfg;
> +
> + if (cfg->flush_walk_prefer_tlbiasid) {
> + arm_smmu_tlb_inv_context_s1(cookie);
Hmm, this introduces an unconditional wmb() if tlbiasid is preferred. I
think that should be predicated on ARM_SMMU_FEAT_COHERENT_WALK like it is
for the by-VA ops. Worth doing as a separate patch.
> + } else {
> + arm_smmu_tlb_inv_range_s1(iova, size, granule, cookie,
> + ARM_SMMU_CB_S1_TLBIVA);
> + arm_smmu_tlb_sync_context(cookie);
> + }
> }
>
> static void arm_smmu_tlb_add_page_s1(struct iommu_iotlb_gather *gather,
> @@ -765,8 +772,10 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
> .iommu_dev = smmu->dev,
> };
>
> - if (!iommu_get_dma_strict(domain))
> + if (!iommu_get_dma_strict(domain)) {
> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> + cfg->flush_walk_prefer_tlbiasid = true;
This is going to interact badly with Robin's series to allow dynamic
transition to non-strict mode, as we don't have a mechanism to switch
over to the by-ASID behaviour. Yes, it should _work_, but it's ugly having
different TLBI behaviour just because of the how the domain became
non-strict.
Robin -- I think this originated from your idea at [1]. Any idea how to make
it work with your other series, or shall we drop this part for now and leave
the TLB invalidation behaviour the same for now?
Will
[1] https://lore.kernel.org/r/da62ff1c-9b49-34d3-69a1-1a674e4a3...@arm.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 2/8] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync()
On Wed, Aug 11, 2021 at 10:16:39AM +0800, Leizhen (ThunderTown) wrote:
>
>
> On 2021/8/11 2:24, Will Deacon wrote:
> > On Sat, Jun 26, 2021 at 07:01:24PM +0800, Zhen Lei wrote:
> >> The obvious key to the performance optimization of commit 587e6c10a7ce
> >> ("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
> >> to allow multiple cores to insert commands in parallel after a brief mutex
> >> contention.
> >>
> >> Obviously, inserting as many commands at a time as possible can reduce the
> >> number of times the mutex contention participates, thereby improving the
> >> overall performance. At least it reduces the number of calls to function
> >> arm_smmu_cmdq_issue_cmdlist().
> >>
> >> Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert
> >> the 'cmd+sync' commands at a time.
> >>
> >> Signed-off-by: Zhen Lei
> >> ---
> >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +
> >> 1 file changed, 21 insertions(+), 12 deletions(-)
> >>
> >> 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 2433d3c29b49ff2..a5361153ca1d6a4 100644
> >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >> @@ -858,11 +858,25 @@ static int arm_smmu_cmdq_issue_cmd(struct
> >> arm_smmu_device *smmu,
> >>return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false);
> >> }
> >>
> >> -static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
> >> +static int __maybe_unused arm_smmu_cmdq_issue_sync(struct arm_smmu_device
> >> *smmu)
> >> {
> >>return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true);
> >> }
> >>
> >> +static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
> >> + struct arm_smmu_cmdq_ent *ent)
> >> +{
> >> + u64 cmd[CMDQ_ENT_DWORDS];
> >> +
> >> + if (arm_smmu_cmdq_build_cmd(cmd, ent)) {
> >> + dev_warn(smmu->dev, "ignoring unknown CMDQ opcode 0x%x\n",
> >> + ent->opcode);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, true);
> >> +}
> >
> > This function is almost identical to arm_smmu_cmdq_issue_cmd(). How about
> > moving the guts out into a helper:
> >
> > static int __arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
> > struct arm_smmu_cmdq_ent *ent,
> > bool sync);
> >
> > and then having arm_smmu_cmdq_issue_cmd_with_sync() and
> > arm_smmu_cmdq_issue_cmd() wrap that?
>
> OK, I will do it.
>
> How about remove arm_smmu_cmdq_issue_sync()? It's unused now.
Sure.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 5/5] dma-iommu: account for min_align_mask
> -Original Message-
> From: iommu On Behalf Of
> David Stevens
> Sent: Wednesday, August 11, 2021 10:43 AM
> To: Robin Murphy ; Will Deacon
> Cc: linux-ker...@vger.kernel.org; Tom Murphy ;
> iommu@lists.linux-foundation.org; David Stevens
> Subject: [PATCH v3 5/5] dma-iommu: account for min_align_mask
>
> From: David Stevens
>
> For devices which set min_align_mask, swiotlb preserves the offset of the
> original physical address within that mask. Since __iommu_dma_map
> accounts for non-aligned addresses, passing a non-aligned swiotlb address
> with the swiotlb aligned size results in the offset being accounted for twice
> in
> the size passed to iommu_map_atomic. The extra page exposed to DMA is
> also not cleaned up by __iommu_dma_unmap, since tht at function unmaps
> with the correct size. This causes mapping failures if the iova gets reused,
> due to collisions in the iommu page tables.
>
> To fix this, pass the original size to __iommu_dma_map, since that function
> already handles alignment.
>
> Additionally, when swiotlb returns non-aligned addresses, there is padding at
> the start of the bounce buffer that needs to be cleared.
>
> Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
> Signed-off-by: David Stevens
> ---
> drivers/iommu/dma-iommu.c | 23 ---
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 89b689bf801f..ffa7e8ef5db4 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -549,9 +549,8 @@ static dma_addr_t
> __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
> struct iommu_domain *domain = iommu_get_dma_domain(dev);
> struct iommu_dma_cookie *cookie = domain->iova_cookie;
> struct iova_domain *iovad = >iovad;
> - size_t aligned_size = org_size;
> - void *padding_start;
> - size_t padding_size;
> + void *tlb_start;
> + size_t aligned_size, iova_off, mapping_end_off;
> dma_addr_t iova;
>
> /*
> @@ -566,24 +565,26 @@ static dma_addr_t
> __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
> if (phys == DMA_MAPPING_ERROR)
> return DMA_MAPPING_ERROR;
>
> - /* Cleanup the padding area. */
> - padding_start = phys_to_virt(phys);
> - padding_size = aligned_size;
> + iova_off = iova_offset(iovad, phys);
> + tlb_start = phys_to_virt(phys - iova_off);
>
> + /* Cleanup the padding area. */
> if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> (dir == DMA_TO_DEVICE ||
>dir == DMA_BIDIRECTIONAL)) {
> - padding_start += org_size;
> - padding_size -= org_size;
> + mapping_end_off = iova_off + org_size;
> + memset(tlb_start, 0, iova_off);
> + memset(tlb_start + mapping_end_off, 0,
> +aligned_size - mapping_end_off);
> + } else {
> + memset(tlb_start, 0, aligned_size);
> }
Nice fix. It's better move the "cleanup ..." comment into if case which looks
more accurate.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv3] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
On 2021-08-10 23:38, Will Deacon wrote:
On Tue, Aug 03, 2021 at 11:09:17AM +0530, Sai Prakash Ranjan wrote:
On 2021-08-02 21:13, Will Deacon wrote:
> On Wed, Jun 23, 2021 at 07:12:01PM +0530, Sai Prakash Ranjan wrote:
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index d3c6f54110a5..f3845e822565 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -341,6 +341,12 @@ static void arm_smmu_tlb_add_page_s1(struct
> > iommu_iotlb_gather *gather,
> > ARM_SMMU_CB_S1_TLBIVAL);
> > }
> >
> > +static void arm_smmu_tlb_inv_walk_impl_s1(unsigned long iova,
> > size_t size,
> > + size_t granule, void *cookie)
> > +{
> > + arm_smmu_tlb_inv_context_s1(cookie);
> > +}
> > +
> > static void arm_smmu_tlb_inv_walk_s2(unsigned long iova, size_t size,
> >size_t granule, void *cookie)
> > {
> > @@ -388,6 +394,12 @@ static const struct iommu_flush_ops
> > arm_smmu_s1_tlb_ops = {
> > .tlb_add_page = arm_smmu_tlb_add_page_s1,
> > };
> >
> > +const struct iommu_flush_ops arm_smmu_s1_tlb_impl_ops = {
> > + .tlb_flush_all = arm_smmu_tlb_inv_context_s1,
> > + .tlb_flush_walk = arm_smmu_tlb_inv_walk_impl_s1,
> > + .tlb_add_page = arm_smmu_tlb_add_page_s1,
> > +};
>
> Hmm, dunno about this. Wouldn't it be a lot cleaner if the
> tlb_flush_walk
> callbacks just did the right thing based on the smmu_domain (maybe in
> the
> arm_smmu_cfg?) rather than having an entirely new set of ops just
> because
> they're const and you can't overide the bit you want?
>
> I don't think there's really an awful lot qcom-specific about the
> principle
> here -- there's a trade-off between over-invalidation and invalidation
> latency. That happens on the CPU as well.
>
Sorry didn't understand, based on smmu_domain what? How do we make
this implementation specific? Do you mean something like a quirk?
The reason we didn't make this common was because nvidia folks weren't
so happy with that, you can find the discussion in this thread [1].
[1]
https://lore.kernel.org/lkml/20210609145315.25750-1-saiprakash.ran...@codeaurora.org/
The ->tlb_flush_walk() callbacks take a 'void *cookie' which, for this
driver, is a 'struct arm_smmu_domain *'. From that, you can get to the
'struct arm_smmu_cfg' which could have something as coarse as:
boolflush_walk_prefer_tlbiasid;
which you can set when you initialise the domain (maybe in the
->init_context callback?). It shouldn't affect anybody else.
Ah ok, you meant a new flag in arm_smmu_cfg, right getting it from
cookie
is no big deal but nonetheless thanks for detailing it. I have made the
changes and sent a v4 after testing.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 3/5] dma-iommu: add SKIP_CPU_SYNC after syncing
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 4f0cc4a0a61f..be0214b1455c 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -859,8 +859,11 @@ static dma_addr_t iommu_dma_map_page(struct device *dev,
> struct page *page,
> static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
> size_t size, enum dma_data_direction dir, unsigned long attrs)
> {
> - if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
> iommu_dma_sync_single_for_cpu(dev, dma_handle, size, dir);
> + attrs |= DMA_ATTR_SKIP_CPU_SYNC;
> + }
> +
> __iommu_dma_unmap_swiotlb(dev, dma_handle, size, dir, attrs);
I don't think this is the correct way to go. Instead just call the
raw cache sync helper instead of iommu_dma_sync_single_for_cpu, similar
to what dma-direct does. Same for the map side.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCHv4] iommu/arm-smmu: Optimize ->tlb_flush_walk() for qcom implementation
Currently for iommu_unmap() of large scatter-gather list with page size
elements, the majority of time is spent in flushing of partial walks in
__arm_lpae_unmap() which is a VA based TLB invalidation invalidating
page-by-page on iommus like arm-smmu-v2 (TLBIVA).
For example: to unmap a 32MB scatter-gather list with page size elements
(8192 entries), there are 16->2MB buffer unmaps based on the pgsize (2MB
for 4K granule) and each of 2MB will further result in 512 TLBIVAs (2MB/4K)
resulting in a total of 8192 TLBIVAs (512*16) for 16->2MB causing a huge
overhead.
On qcom implementation, there are several performance improvements for
TLB cache invalidations in HW like wait-for-safe (for realtime clients
such as camera and display) and few others to allow for cache
lookups/updates when TLBI is in progress for the same context bank.
So the cost of over-invalidation is less compared to the unmap latency
on several usecases like camera which deals with large buffers. So,
ASID based TLB invalidations (TLBIASID) can be used to invalidate the
entire context for partial walk flush thereby improving the unmap
latency.
Non-strict mode can use this by default for all platforms given its
all about over-invalidation saving time on individual unmaps and
non-deterministic generally.
For this example of 32MB scatter-gather list unmap, this change results
in just 16 ASID based TLB invalidations (TLBIASIDs) as opposed to 8192
TLBIVAs thereby increasing the performance of unmaps drastically.
Test on QTI SM8150 SoC for 10 iterations of iommu_{map_sg}/unmap:
(average over 10 iterations)
Before this optimization:
sizeiommu_map_sg iommu_unmap
4K2.067 us 1.854 us
64K9.598 us 8.802 us
1M 148.890 us 130.718 us
2M 305.864 us67.291 us
12M 1793.604 us 390.838 us
16M 2386.848 us 518.187 us
24M 3563.296 us 775.989 us
32M 4747.171 us 1033.364 us
After this optimization:
sizeiommu_map_sg iommu_unmap
4K1.723 us 1.765 us
64K9.880 us 8.869 us
1M 155.364 us 135.223 us
2M 303.906 us 5.385 us
12M 1786.557 us21.250 us
16M 2391.890 us27.437 us
24M 3570.895 us39.937 us
32M 4755.234 us51.797 us
Real world data also shows big difference in unmap performance as below:
There were reports of camera frame drops because of high overhead in
iommu unmap without this optimization because of frequent unmaps issued
by camera of about 100MB/s taking more than 100ms thereby causing frame
drops.
Signed-off-by: Sai Prakash Ranjan
---
Changes in v4:
* Use a flag in struct arm_smmu_cfg to prefer TLBIASID (Will)
Changes in v3:
* Move the logic to arm-smmu driver from io-pgtable (Robin)
* Use a new set of iommu_flush_ops->arm_smmu_s1_tlb_impl_ops and use it for
qcom impl
Changes in v2:
* Add a quirk to choose tlb_flush_all in partial walk flush
* Set the quirk for QTI SoC implementation
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 11 +++
drivers/iommu/arm/arm-smmu/arm-smmu.c | 17 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 9b9d13ec5a88..55690af1b25d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -193,6 +193,8 @@ static int qcom_adreno_smmu_init_context(struct
arm_smmu_domain *smmu_domain,
{
struct adreno_smmu_priv *priv;
+ smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
+
/* Only enable split pagetables for the GPU device (SID 0) */
if (!qcom_adreno_smmu_is_gpu_device(dev))
return 0;
@@ -235,6 +237,14 @@ static const struct of_device_id
qcom_smmu_client_of_match[] __maybe_unused = {
{ }
};
+static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
+ struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
+{
+ smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
+
+ return 0;
+}
+
static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
{
unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups -
1);
@@ -358,6 +368,7 @@ static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
}
static const struct arm_smmu_impl qcom_smmu_impl = {
+ .init_context = qcom_smmu_init_context,
.cfg_probe = qcom_smmu_cfg_probe,
.def_domain_type = qcom_smmu_def_domain_type,
.reset = qcom_smmu500_reset,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f7da8953afbe..3904b598e0f9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
Re: [PATCH v3 2/5] dma-iommu: fix arch_sync_dma for map
Looks good, Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
