Re: [PATCH 4/5] iommu: Separate IOVA rcache memories from iova_domain structure
On 20/12/2021 13:57, Robin Murphy wrote: Do you have any thoughts on this patch? The decision is whether we stick with a single iova domain structure or support this super structure for iova domains which support the rcache. I did not try the former - it would be do-able but I am not sure on how it would look. TBH I feel inclined to take the simpler approach of just splitting the rcache array to a separate allocation, making init_iova_rcaches() public (with a proper return value), and tweaking put_iova_domain() to make rcache cleanup conditional. A residual overhead of 3 extra pointers in iova_domain doesn't seem like *too* much for non-DMA-API users to bear. OK, fine. So I tried as you suggested and it looks ok to me. I'll send something out at rc1. Unless you want to try generalising the rcache mechanism completely away from IOVA API specifics, it doesn't seem like there's really enough to justify the bother of having its own distinct abstraction layer. Yeah, I don't see that as necessary. However something which could be useful is to separate the magazine code out for other possible users. Thanks! John ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] iommu: Separate IOVA rcache memories from iova_domain structure
Hi John, On 2021-12-20 08:49, John Garry wrote: On 24/09/2021 11:01, John Garry wrote: Only dma-iommu.c and vdpa actually use the "fast" mode of IOVA alloc and free. As such, it's wasteful that all other IOVA domains hold the rcache memories. In addition, the current IOVA domain init implementation is poor (init_iova_domain()), in that errors are ignored and not passed to the caller. The only errors can come from the IOVA rcache init, and fixing up all the IOVA domain init callsites to handle the errors would take some work. Separate the IOVA rache out of the IOVA domain, and create a new IOVA domain structure, iova_caching_domain. Signed-off-by: John Garry Hi Robin, Do you have any thoughts on this patch? The decision is whether we stick with a single iova domain structure or support this super structure for iova domains which support the rcache. I did not try the former - it would be do-able but I am not sure on how it would look. TBH I feel inclined to take the simpler approach of just splitting the rcache array to a separate allocation, making init_iova_rcaches() public (with a proper return value), and tweaking put_iova_domain() to make rcache cleanup conditional. A residual overhead of 3 extra pointers in iova_domain doesn't seem like *too* much for non-DMA-API users to bear. Unless you want to try generalising the rcache mechanism completely away from IOVA API specifics, it doesn't seem like there's really enough to justify the bother of having its own distinct abstraction layer. Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] iommu: Separate IOVA rcache memories from iova_domain structure
On 24/09/2021 11:01, John Garry wrote:
Only dma-iommu.c and vdpa actually use the "fast" mode of IOVA alloc and
free. As such, it's wasteful that all other IOVA domains hold the rcache
memories.
In addition, the current IOVA domain init implementation is poor
(init_iova_domain()), in that errors are ignored and not passed to the
caller. The only errors can come from the IOVA rcache init, and fixing up
all the IOVA domain init callsites to handle the errors would take some
work.
Separate the IOVA rache out of the IOVA domain, and create a new IOVA
domain structure, iova_caching_domain.
Signed-off-by: John Garry
Hi Robin,
Do you have any thoughts on this patch? The decision is whether we stick
with a single iova domain structure or support this super structure for
iova domains which support the rcache. I did not try the former - it
would be do-able but I am not sure on how it would look.
Thanks,
John
EOM
---
drivers/iommu/dma-iommu.c| 56 +++-
drivers/iommu/iova.c | 125 ++-
drivers/vdpa/vdpa_user/iova_domain.c | 53 +++-
drivers/vdpa/vdpa_user/iova_domain.h | 4 +-
include/linux/iova.h | 18 ++--
5 files changed, 166 insertions(+), 90 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index fd669bad96e1..70651f1a688d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -92,8 +92,8 @@ struct iommu_dma_cookie {
union {
/* Full allocator for IOMMU_DMA_IOVA_COOKIE */
struct {
- struct iova_domain iovad;
- struct fq_domainfq;
+ struct iova_caching_domain rcached;
+ struct fq_domainfq;
};
/* Trivial linear page allocator for IOMMU_DMA_MSI_COOKIE */
dma_addr_t msi_iova;
@@ -197,7 +197,6 @@ static void fq_ring_free(struct fq_domain *fq_domain,
struct fq *fq)
struct iommu_dma_cookie *cookie = container_of(fq_domain,
struct iommu_dma_cookie,
fq);
- struct iova_domain *iovad = &cookie->iovad;
u64 counter = atomic64_read(&fq_domain->fq_flush_finish_cnt);
unsigned idx;
@@ -211,7 +210,7 @@ static void fq_ring_free(struct fq_domain *fq_domain, struct fq *fq)
if (fq_domain->entry_dtor)
fq_domain->entry_dtor(fq->entries[idx].data);
- free_iova_fast(iovad,
+ free_iova_fast(&cookie->rcached,
fq->entries[idx].iova_pfn,
fq->entries[idx].pages);
@@ -330,7 +329,7 @@ static int init_flush_queue(struct fq_domain *fq_domain,
static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
{
if (cookie->type == IOMMU_DMA_IOVA_COOKIE)
- return cookie->iovad.granule;
+ return cookie->rcached.iovad.granule;
return PAGE_SIZE;
}
@@ -413,9 +412,10 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (!cookie)
return;
- if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule) {
+ if (cookie->type == IOMMU_DMA_IOVA_COOKIE &&
+ cookie->rcached.iovad.granule) {
free_flush_queue(&cookie->fq);
- put_iova_domain(&cookie->iovad);
+ put_iova_caching_domain(&cookie->rcached);
}
list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
@@ -449,7 +449,7 @@ EXPORT_SYMBOL(iommu_dma_get_resv_regions);
static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie,
phys_addr_t start, phys_addr_t end)
{
- struct iova_domain *iovad = &cookie->iovad;
+ struct iova_domain *iovad = &cookie->rcached.iovad;
struct iommu_dma_msi_page *msi_page;
int i, num_pages;
@@ -520,7 +520,8 @@ static int iova_reserve_iommu_regions(struct device *dev,
struct iommu_domain *domain)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
- struct iova_domain *iovad = &cookie->iovad;
+ struct iova_caching_domain *rcached = &cookie->rcached;
+ struct iova_domain *iovad = &rcached->iovad;
struct iommu_resv_region *region;
LIST_HEAD(resv_regions);
int ret = 0;
@@ -612,14 +613,17 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
dma_addr_t limit, struct device *dev)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iova_caching_domain *rcached;
unsigned long order, base_pfn;
struct iova_domain *iovad;
struct fq_domain *fq;
+ int ret;
if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
[PATCH 4/5] iommu: Separate IOVA rcache memories from iova_domain structure
Only dma-iommu.c and vdpa actually use the "fast" mode of IOVA alloc and
free. As such, it's wasteful that all other IOVA domains hold the rcache
memories.
In addition, the current IOVA domain init implementation is poor
(init_iova_domain()), in that errors are ignored and not passed to the
caller. The only errors can come from the IOVA rcache init, and fixing up
all the IOVA domain init callsites to handle the errors would take some
work.
Separate the IOVA rache out of the IOVA domain, and create a new IOVA
domain structure, iova_caching_domain.
Signed-off-by: John Garry
---
drivers/iommu/dma-iommu.c| 56 +++-
drivers/iommu/iova.c | 125 ++-
drivers/vdpa/vdpa_user/iova_domain.c | 53 +++-
drivers/vdpa/vdpa_user/iova_domain.h | 4 +-
include/linux/iova.h | 18 ++--
5 files changed, 166 insertions(+), 90 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index fd669bad96e1..70651f1a688d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -92,8 +92,8 @@ struct iommu_dma_cookie {
union {
/* Full allocator for IOMMU_DMA_IOVA_COOKIE */
struct {
- struct iova_domain iovad;
- struct fq_domainfq;
+ struct iova_caching_domain rcached;
+ struct fq_domainfq;
};
/* Trivial linear page allocator for IOMMU_DMA_MSI_COOKIE */
dma_addr_t msi_iova;
@@ -197,7 +197,6 @@ static void fq_ring_free(struct fq_domain *fq_domain,
struct fq *fq)
struct iommu_dma_cookie *cookie = container_of(fq_domain,
struct iommu_dma_cookie,
fq);
- struct iova_domain *iovad = &cookie->iovad;
u64 counter = atomic64_read(&fq_domain->fq_flush_finish_cnt);
unsigned idx;
@@ -211,7 +210,7 @@ static void fq_ring_free(struct fq_domain *fq_domain,
struct fq *fq)
if (fq_domain->entry_dtor)
fq_domain->entry_dtor(fq->entries[idx].data);
- free_iova_fast(iovad,
+ free_iova_fast(&cookie->rcached,
fq->entries[idx].iova_pfn,
fq->entries[idx].pages);
@@ -330,7 +329,7 @@ static int init_flush_queue(struct fq_domain *fq_domain,
static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
{
if (cookie->type == IOMMU_DMA_IOVA_COOKIE)
- return cookie->iovad.granule;
+ return cookie->rcached.iovad.granule;
return PAGE_SIZE;
}
@@ -413,9 +412,10 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (!cookie)
return;
- if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule) {
+ if (cookie->type == IOMMU_DMA_IOVA_COOKIE &&
+ cookie->rcached.iovad.granule) {
free_flush_queue(&cookie->fq);
- put_iova_domain(&cookie->iovad);
+ put_iova_caching_domain(&cookie->rcached);
}
list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
@@ -449,7 +449,7 @@ EXPORT_SYMBOL(iommu_dma_get_resv_regions);
static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie,
phys_addr_t start, phys_addr_t end)
{
- struct iova_domain *iovad = &cookie->iovad;
+ struct iova_domain *iovad = &cookie->rcached.iovad;
struct iommu_dma_msi_page *msi_page;
int i, num_pages;
@@ -520,7 +520,8 @@ static int iova_reserve_iommu_regions(struct device *dev,
struct iommu_domain *domain)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
- struct iova_domain *iovad = &cookie->iovad;
+ struct iova_caching_domain *rcached = &cookie->rcached;
+ struct iova_domain *iovad = &rcached->iovad;
struct iommu_resv_region *region;
LIST_HEAD(resv_regions);
int ret = 0;
@@ -612,14 +613,17 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
dma_addr_t limit, struct device *dev)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iova_caching_domain *rcached;
unsigned long order, base_pfn;
struct iova_domain *iovad;
struct fq_domain *fq;
+ int ret;
if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
return -EINVAL;
- iovad = &cookie->iovad;
+ rcached = &cookie->rcached;
+ iovad = &rcached->iovad;
fq = &cookie->fq;
/* Use the smallest supported page size for IOVA granularity */
@@ -652,7 +656,11 @@ static int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
fq->flush_c
