* [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 10:54 ` Juergen Gross
0 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 10:54 UTC (permalink / raw)
To: xen-devel, linux-kernel, iommu
Cc: Juergen Gross, boris.ostrovsky, sstabellini, konrad.wilk
Instead of always calling xen_destroy_contiguous_region() in case the
memory is DMA-able for the used device, do so only in case it has been
made DMA-able via xen_create_contiguous_region() before.
This will avoid a lot of xen_destroy_contiguous_region() calls for
64-bit capable devices.
As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
flag of the first allocated page can be used for remembering.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/swiotlb-xen.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 43b6e65ae256..a72f181d8e20 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
return NULL;
}
+ SetPageOwnerPriv1(virt_to_page(ret));
}
memset(ret, 0, size);
return ret;
@@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
/* Convert the size to actually allocated. */
size = 1UL << (order + XEN_PAGE_SHIFT);
- if ((dev_addr + size - 1 <= dma_mask) &&
- !WARN_ON(range_straddles_page_boundary(phys, size)))
- xen_destroy_contiguous_region(phys, order);
+ if (PageOwnerPriv1(virt_to_page(vaddr))) {
+ if (!WARN_ON(range_straddles_page_boundary(phys, size)))
+ xen_destroy_contiguous_region(phys, order);
+ ClearPageOwnerPriv1(virt_to_page(vaddr));
+ }
xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
}
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 10:54 ` Juergen Gross
0 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 10:54 UTC (permalink / raw)
To: xen-devel, linux-kernel, iommu
Cc: Juergen Gross, boris.ostrovsky, sstabellini, konrad.wilk
Instead of always calling xen_destroy_contiguous_region() in case the
memory is DMA-able for the used device, do so only in case it has been
made DMA-able via xen_create_contiguous_region() before.
This will avoid a lot of xen_destroy_contiguous_region() calls for
64-bit capable devices.
As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
flag of the first allocated page can be used for remembering.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/swiotlb-xen.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 43b6e65ae256..a72f181d8e20 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
return NULL;
}
+ SetPageOwnerPriv1(virt_to_page(ret));
}
memset(ret, 0, size);
return ret;
@@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
/* Convert the size to actually allocated. */
size = 1UL << (order + XEN_PAGE_SHIFT);
- if ((dev_addr + size - 1 <= dma_mask) &&
- !WARN_ON(range_straddles_page_boundary(phys, size)))
- xen_destroy_contiguous_region(phys, order);
+ if (PageOwnerPriv1(virt_to_page(vaddr))) {
+ if (!WARN_ON(range_straddles_page_boundary(phys, size)))
+ xen_destroy_contiguous_region(phys, order);
+ ClearPageOwnerPriv1(virt_to_page(vaddr));
+ }
xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
}
--
2.16.4
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 10:54 ` Juergen Gross
0 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 10:54 UTC (permalink / raw)
To: xen-devel-GuqFBffKawtpuQazS67q72D2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Cc: Juergen Gross, boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA,
sstabellini-DgEjT+Ai2ygdnm+yROfE0A,
konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA
Instead of always calling xen_destroy_contiguous_region() in case the
memory is DMA-able for the used device, do so only in case it has been
made DMA-able via xen_create_contiguous_region() before.
This will avoid a lot of xen_destroy_contiguous_region() calls for
64-bit capable devices.
As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
flag of the first allocated page can be used for remembering.
Signed-off-by: Juergen Gross <jgross-IBi9RG/b67k@public.gmane.org>
---
drivers/xen/swiotlb-xen.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 43b6e65ae256..a72f181d8e20 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
return NULL;
}
+ SetPageOwnerPriv1(virt_to_page(ret));
}
memset(ret, 0, size);
return ret;
@@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
/* Convert the size to actually allocated. */
size = 1UL << (order + XEN_PAGE_SHIFT);
- if ((dev_addr + size - 1 <= dma_mask) &&
- !WARN_ON(range_straddles_page_boundary(phys, size)))
- xen_destroy_contiguous_region(phys, order);
+ if (PageOwnerPriv1(virt_to_page(vaddr))) {
+ if (!WARN_ON(range_straddles_page_boundary(phys, size)))
+ xen_destroy_contiguous_region(phys, order);
+ ClearPageOwnerPriv1(virt_to_page(vaddr));
+ }
xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
}
--
2.16.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 14:31 ` Boris Ostrovsky
0 siblings, 0 replies; 48+ messages in thread
From: Boris Ostrovsky @ 2019-04-23 14:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-kernel, iommu; +Cc: konrad.wilk, sstabellini
On 4/23/19 6:54 AM, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
I think a new enum in pageflags would be useful, and be consistent with
other flag uses.
-boris
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 14:31 ` Boris Ostrovsky
0 siblings, 0 replies; 48+ messages in thread
From: Boris Ostrovsky @ 2019-04-23 14:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-kernel, iommu; +Cc: sstabellini, konrad.wilk
On 4/23/19 6:54 AM, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
I think a new enum in pageflags would be useful, and be consistent with
other flag uses.
-boris
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 14:31 ` Boris Ostrovsky
0 siblings, 0 replies; 48+ messages in thread
From: Boris Ostrovsky @ 2019-04-23 14:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-kernel, iommu; +Cc: sstabellini, konrad.wilk
On 4/23/19 6:54 AM, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
I think a new enum in pageflags would be useful, and be consistent with
other flag uses.
-boris
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 14:31 ` Boris Ostrovsky
0 siblings, 0 replies; 48+ messages in thread
From: Boris Ostrovsky @ 2019-04-23 14:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel-GuqFBffKawtpuQazS67q72D2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Cc: sstabellini-DgEjT+Ai2ygdnm+yROfE0A, konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA
On 4/23/19 6:54 AM, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
I think a new enum in pageflags would be useful, and be consistent with
other flag uses.
-boris
>
> Signed-off-by: Juergen Gross <jgross-IBi9RG/b67k@public.gmane.org>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
2019-04-23 10:54 ` Juergen Gross
` (3 preceding siblings ...)
(?)
@ 2019-04-23 14:31 ` Boris Ostrovsky
-1 siblings, 0 replies; 48+ messages in thread
From: Boris Ostrovsky @ 2019-04-23 14:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-kernel, iommu; +Cc: sstabellini, konrad.wilk
On 4/23/19 6:54 AM, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
I think a new enum in pageflags would be useful, and be consistent with
other flag uses.
-boris
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
2019-04-23 10:54 ` Juergen Gross
` (4 preceding siblings ...)
(?)
@ 2019-04-23 17:05 ` Stefano Stabellini
-1 siblings, 0 replies; 48+ messages in thread
From: Stefano Stabellini @ 2019-04-23 17:05 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, konrad.wilk, linux-kernel, iommu, xen-devel,
boris.ostrovsky
On Tue, 23 Apr 2019, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
Although the patch looks OK, this sentence puzzles me. Why do you say
that the memory in question is owned by swiotlb-xen? Because it was
returned by xen_alloc_coherent_pages? Both the x86 and the Arm
implementation return fresh new memory, hence, it should be safe to set
the PageOwnerPriv1 flag?
My concern with this approach is with the semantics of PG_owner_priv_1.
Is a page marked with PG_owner_priv_1 only supposed to be used by the
owner?
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 17:05 ` Stefano Stabellini
0 siblings, 0 replies; 48+ messages in thread
From: Stefano Stabellini @ 2019-04-23 17:05 UTC (permalink / raw)
To: Juergen Gross
Cc: xen-devel, linux-kernel, iommu, konrad.wilk, boris.ostrovsky,
sstabellini
On Tue, 23 Apr 2019, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
Although the patch looks OK, this sentence puzzles me. Why do you say
that the memory in question is owned by swiotlb-xen? Because it was
returned by xen_alloc_coherent_pages? Both the x86 and the Arm
implementation return fresh new memory, hence, it should be safe to set
the PageOwnerPriv1 flag?
My concern with this approach is with the semantics of PG_owner_priv_1.
Is a page marked with PG_owner_priv_1 only supposed to be used by the
owner?
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 17:05 ` Stefano Stabellini
0 siblings, 0 replies; 48+ messages in thread
From: Stefano Stabellini @ 2019-04-23 17:05 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, konrad.wilk, linux-kernel, iommu, xen-devel,
boris.ostrovsky
On Tue, 23 Apr 2019, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
Although the patch looks OK, this sentence puzzles me. Why do you say
that the memory in question is owned by swiotlb-xen? Because it was
returned by xen_alloc_coherent_pages? Both the x86 and the Arm
implementation return fresh new memory, hence, it should be safe to set
the PageOwnerPriv1 flag?
My concern with this approach is with the semantics of PG_owner_priv_1.
Is a page marked with PG_owner_priv_1 only supposed to be used by the
owner?
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 17:05 ` Stefano Stabellini
0 siblings, 0 replies; 48+ messages in thread
From: Stefano Stabellini @ 2019-04-23 17:05 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, konrad.wilk, linux-kernel, iommu, xen-devel,
boris.ostrovsky
On Tue, 23 Apr 2019, Juergen Gross wrote:
> Instead of always calling xen_destroy_contiguous_region() in case the
> memory is DMA-able for the used device, do so only in case it has been
> made DMA-able via xen_create_contiguous_region() before.
>
> This will avoid a lot of xen_destroy_contiguous_region() calls for
> 64-bit capable devices.
>
> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
> flag of the first allocated page can be used for remembering.
Although the patch looks OK, this sentence puzzles me. Why do you say
that the memory in question is owned by swiotlb-xen? Because it was
returned by xen_alloc_coherent_pages? Both the x86 and the Arm
implementation return fresh new memory, hence, it should be safe to set
the PageOwnerPriv1 flag?
My concern with this approach is with the semantics of PG_owner_priv_1.
Is a page marked with PG_owner_priv_1 only supposed to be used by the
owner?
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/swiotlb-xen.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 43b6e65ae256..a72f181d8e20 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -321,6 +321,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
> + SetPageOwnerPriv1(virt_to_page(ret));
> }
> memset(ret, 0, size);
> return ret;
> @@ -344,9 +345,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> /* Convert the size to actually allocated. */
> size = 1UL << (order + XEN_PAGE_SHIFT);
>
> - if ((dev_addr + size - 1 <= dma_mask) &&
> - !WARN_ON(range_straddles_page_boundary(phys, size)))
> - xen_destroy_contiguous_region(phys, order);
> + if (PageOwnerPriv1(virt_to_page(vaddr))) {
> + if (!WARN_ON(range_straddles_page_boundary(phys, size)))
> + xen_destroy_contiguous_region(phys, order);
> + ClearPageOwnerPriv1(virt_to_page(vaddr));
> + }
>
> xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 18:36 ` Juergen Gross
0 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 18:36 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, linux-kernel, iommu, konrad.wilk, boris.ostrovsky
On 23/04/2019 19:05, Stefano Stabellini wrote:
> On Tue, 23 Apr 2019, Juergen Gross wrote:
>> Instead of always calling xen_destroy_contiguous_region() in case the
>> memory is DMA-able for the used device, do so only in case it has been
>> made DMA-able via xen_create_contiguous_region() before.
>>
>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>> 64-bit capable devices.
>>
>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>> flag of the first allocated page can be used for remembering.
>
> Although the patch looks OK, this sentence puzzles me. Why do you say
> that the memory in question is owned by swiotlb-xen? Because it was
> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
> implementation return fresh new memory, hence, it should be safe to set
> the PageOwnerPriv1 flag?
>
> My concern with this approach is with the semantics of PG_owner_priv_1.
> Is a page marked with PG_owner_priv_1 only supposed to be used by the
> owner?
The owner of the page is free to use the flag.
Like Grant pages are marked by the grant driver using this flag. And
Xen page tables are using it in PV-guests for indicating a "Pinned"
page table.
Juergen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 18:36 ` Juergen Gross
0 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 18:36 UTC (permalink / raw)
To: Stefano Stabellini
Cc: boris.ostrovsky, xen-devel, iommu, linux-kernel, konrad.wilk
On 23/04/2019 19:05, Stefano Stabellini wrote:
> On Tue, 23 Apr 2019, Juergen Gross wrote:
>> Instead of always calling xen_destroy_contiguous_region() in case the
>> memory is DMA-able for the used device, do so only in case it has been
>> made DMA-able via xen_create_contiguous_region() before.
>>
>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>> 64-bit capable devices.
>>
>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>> flag of the first allocated page can be used for remembering.
>
> Although the patch looks OK, this sentence puzzles me. Why do you say
> that the memory in question is owned by swiotlb-xen? Because it was
> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
> implementation return fresh new memory, hence, it should be safe to set
> the PageOwnerPriv1 flag?
>
> My concern with this approach is with the semantics of PG_owner_priv_1.
> Is a page marked with PG_owner_priv_1 only supposed to be used by the
> owner?
The owner of the page is free to use the flag.
Like Grant pages are marked by the grant driver using this flag. And
Xen page tables are using it in PV-guests for indicating a "Pinned"
page table.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-23 18:36 ` Juergen Gross
0 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 18:36 UTC (permalink / raw)
To: Stefano Stabellini
Cc: boris.ostrovsky, xen-devel, iommu, linux-kernel, konrad.wilk
On 23/04/2019 19:05, Stefano Stabellini wrote:
> On Tue, 23 Apr 2019, Juergen Gross wrote:
>> Instead of always calling xen_destroy_contiguous_region() in case the
>> memory is DMA-able for the used device, do so only in case it has been
>> made DMA-able via xen_create_contiguous_region() before.
>>
>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>> 64-bit capable devices.
>>
>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>> flag of the first allocated page can be used for remembering.
>
> Although the patch looks OK, this sentence puzzles me. Why do you say
> that the memory in question is owned by swiotlb-xen? Because it was
> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
> implementation return fresh new memory, hence, it should be safe to set
> the PageOwnerPriv1 flag?
>
> My concern with this approach is with the semantics of PG_owner_priv_1.
> Is a page marked with PG_owner_priv_1 only supposed to be used by the
> owner?
The owner of the page is free to use the flag.
Like Grant pages are marked by the grant driver using this flag. And
Xen page tables are using it in PV-guests for indicating a "Pinned"
page table.
Juergen
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
2019-04-23 18:36 ` Juergen Gross
(?)
(?)
@ 2019-04-25 9:01 ` Jan Beulich
-1 siblings, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2019-04-25 9:01 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, linux-kernel, iommu,
xen-devel, Boris Ostrovsky
>>> On 23.04.19 at 20:36, <jgross@suse.com> wrote:
> On 23/04/2019 19:05, Stefano Stabellini wrote:
>> On Tue, 23 Apr 2019, Juergen Gross wrote:
>>> Instead of always calling xen_destroy_contiguous_region() in case the
>>> memory is DMA-able for the used device, do so only in case it has been
>>> made DMA-able via xen_create_contiguous_region() before.
>>>
>>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>>> 64-bit capable devices.
>>>
>>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>>> flag of the first allocated page can be used for remembering.
>>
>> Although the patch looks OK, this sentence puzzles me. Why do you say
>> that the memory in question is owned by swiotlb-xen? Because it was
>> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
>> implementation return fresh new memory, hence, it should be safe to set
>> the PageOwnerPriv1 flag?
>>
>> My concern with this approach is with the semantics of PG_owner_priv_1.
>> Is a page marked with PG_owner_priv_1 only supposed to be used by the
>> owner?
>
> The owner of the page is free to use the flag.
>
> Like Grant pages are marked by the grant driver using this flag. And
> Xen page tables are using it in PV-guests for indicating a "Pinned"
> page table.
Considering the background of the series, isn't such multi-purpose use
of the flag a possible problem? You're already suspecting a wrong call
into here. The function finding the flag set (but for another reason)
might add to the confusion. But I realize there are only so many page
flags available.
Perhaps the freeing function should, first thing, check the handed
space actually matches the criteria (within dma_mask and contiguous)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
2019-04-23 18:36 ` Juergen Gross
(?)
(?)
@ 2019-04-25 9:01 ` Jan Beulich
-1 siblings, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2019-04-25 9:01 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, iommu, xen-devel, Boris Ostrovsky,
Konrad Rzeszutek Wilk, linux-kernel
>>> On 23.04.19 at 20:36, <jgross@suse.com> wrote:
> On 23/04/2019 19:05, Stefano Stabellini wrote:
>> On Tue, 23 Apr 2019, Juergen Gross wrote:
>>> Instead of always calling xen_destroy_contiguous_region() in case the
>>> memory is DMA-able for the used device, do so only in case it has been
>>> made DMA-able via xen_create_contiguous_region() before.
>>>
>>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>>> 64-bit capable devices.
>>>
>>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>>> flag of the first allocated page can be used for remembering.
>>
>> Although the patch looks OK, this sentence puzzles me. Why do you say
>> that the memory in question is owned by swiotlb-xen? Because it was
>> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
>> implementation return fresh new memory, hence, it should be safe to set
>> the PageOwnerPriv1 flag?
>>
>> My concern with this approach is with the semantics of PG_owner_priv_1.
>> Is a page marked with PG_owner_priv_1 only supposed to be used by the
>> owner?
>
> The owner of the page is free to use the flag.
>
> Like Grant pages are marked by the grant driver using this flag. And
> Xen page tables are using it in PV-guests for indicating a "Pinned"
> page table.
Considering the background of the series, isn't such multi-purpose use
of the flag a possible problem? You're already suspecting a wrong call
into here. The function finding the flag set (but for another reason)
might add to the confusion. But I realize there are only so many page
flags available.
Perhaps the freeing function should, first thing, check the handed
space actually matches the criteria (within dma_mask and contiguous)?
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-25 9:01 ` Jan Beulich
0 siblings, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2019-04-25 9:01 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, linux-kernel, iommu,
xen-devel, Boris Ostrovsky
>>> On 23.04.19 at 20:36, <jgross@suse.com> wrote:
> On 23/04/2019 19:05, Stefano Stabellini wrote:
>> On Tue, 23 Apr 2019, Juergen Gross wrote:
>>> Instead of always calling xen_destroy_contiguous_region() in case the
>>> memory is DMA-able for the used device, do so only in case it has been
>>> made DMA-able via xen_create_contiguous_region() before.
>>>
>>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>>> 64-bit capable devices.
>>>
>>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>>> flag of the first allocated page can be used for remembering.
>>
>> Although the patch looks OK, this sentence puzzles me. Why do you say
>> that the memory in question is owned by swiotlb-xen? Because it was
>> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
>> implementation return fresh new memory, hence, it should be safe to set
>> the PageOwnerPriv1 flag?
>>
>> My concern with this approach is with the semantics of PG_owner_priv_1.
>> Is a page marked with PG_owner_priv_1 only supposed to be used by the
>> owner?
>
> The owner of the page is free to use the flag.
>
> Like Grant pages are marked by the grant driver using this flag. And
> Xen page tables are using it in PV-guests for indicating a "Pinned"
> page table.
Considering the background of the series, isn't such multi-purpose use
of the flag a possible problem? You're already suspecting a wrong call
into here. The function finding the flag set (but for another reason)
might add to the confusion. But I realize there are only so many page
flags available.
Perhaps the freeing function should, first thing, check the handed
space actually matches the criteria (within dma_mask and contiguous)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-25 9:01 ` Jan Beulich
0 siblings, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2019-04-25 9:01 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, linux-kernel, iommu,
xen-devel, Boris Ostrovsky
>>> On 23.04.19 at 20:36, <jgross@suse.com> wrote:
> On 23/04/2019 19:05, Stefano Stabellini wrote:
>> On Tue, 23 Apr 2019, Juergen Gross wrote:
>>> Instead of always calling xen_destroy_contiguous_region() in case the
>>> memory is DMA-able for the used device, do so only in case it has been
>>> made DMA-able via xen_create_contiguous_region() before.
>>>
>>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>>> 64-bit capable devices.
>>>
>>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>>> flag of the first allocated page can be used for remembering.
>>
>> Although the patch looks OK, this sentence puzzles me. Why do you say
>> that the memory in question is owned by swiotlb-xen? Because it was
>> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
>> implementation return fresh new memory, hence, it should be safe to set
>> the PageOwnerPriv1 flag?
>>
>> My concern with this approach is with the semantics of PG_owner_priv_1.
>> Is a page marked with PG_owner_priv_1 only supposed to be used by the
>> owner?
>
> The owner of the page is free to use the flag.
>
> Like Grant pages are marked by the grant driver using this flag. And
> Xen page tables are using it in PV-guests for indicating a "Pinned"
> page table.
Considering the background of the series, isn't such multi-purpose use
of the flag a possible problem? You're already suspecting a wrong call
into here. The function finding the flag set (but for another reason)
might add to the confusion. But I realize there are only so many page
flags available.
Perhaps the freeing function should, first thing, check the handed
space actually matches the criteria (within dma_mask and contiguous)?
Jan
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
@ 2019-04-25 9:01 ` Jan Beulich
0 siblings, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2019-04-25 9:01 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, iommu, xen-devel, Boris Ostrovsky,
Konrad Rzeszutek Wilk, linux-kernel
>>> On 23.04.19 at 20:36, <jgross@suse.com> wrote:
> On 23/04/2019 19:05, Stefano Stabellini wrote:
>> On Tue, 23 Apr 2019, Juergen Gross wrote:
>>> Instead of always calling xen_destroy_contiguous_region() in case the
>>> memory is DMA-able for the used device, do so only in case it has been
>>> made DMA-able via xen_create_contiguous_region() before.
>>>
>>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>>> 64-bit capable devices.
>>>
>>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>>> flag of the first allocated page can be used for remembering.
>>
>> Although the patch looks OK, this sentence puzzles me. Why do you say
>> that the memory in question is owned by swiotlb-xen? Because it was
>> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
>> implementation return fresh new memory, hence, it should be safe to set
>> the PageOwnerPriv1 flag?
>>
>> My concern with this approach is with the semantics of PG_owner_priv_1.
>> Is a page marked with PG_owner_priv_1 only supposed to be used by the
>> owner?
>
> The owner of the page is free to use the flag.
>
> Like Grant pages are marked by the grant driver using this flag. And
> Xen page tables are using it in PV-guests for indicating a "Pinned"
> page table.
Considering the background of the series, isn't such multi-purpose use
of the flag a possible problem? You're already suspecting a wrong call
into here. The function finding the flag set (but for another reason)
might add to the confusion. But I realize there are only so many page
flags available.
Perhaps the freeing function should, first thing, check the handed
space actually matches the criteria (within dma_mask and contiguous)?
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/3] xen/swiotlb: remember having called xen_create_contiguous_region()
2019-04-23 17:05 ` Stefano Stabellini
` (2 preceding siblings ...)
(?)
@ 2019-04-23 18:36 ` Juergen Gross
-1 siblings, 0 replies; 48+ messages in thread
From: Juergen Gross @ 2019-04-23 18:36 UTC (permalink / raw)
To: Stefano Stabellini
Cc: boris.ostrovsky, xen-devel, iommu, linux-kernel, konrad.wilk
On 23/04/2019 19:05, Stefano Stabellini wrote:
> On Tue, 23 Apr 2019, Juergen Gross wrote:
>> Instead of always calling xen_destroy_contiguous_region() in case the
>> memory is DMA-able for the used device, do so only in case it has been
>> made DMA-able via xen_create_contiguous_region() before.
>>
>> This will avoid a lot of xen_destroy_contiguous_region() calls for
>> 64-bit capable devices.
>>
>> As the memory in question is owned by swiotlb-xen the PG_owner_priv_1
>> flag of the first allocated page can be used for remembering.
>
> Although the patch looks OK, this sentence puzzles me. Why do you say
> that the memory in question is owned by swiotlb-xen? Because it was
> returned by xen_alloc_coherent_pages? Both the x86 and the Arm
> implementation return fresh new memory, hence, it should be safe to set
> the PageOwnerPriv1 flag?
>
> My concern with this approach is with the semantics of PG_owner_priv_1.
> Is a page marked with PG_owner_priv_1 only supposed to be used by the
> owner?
The owner of the page is free to use the flag.
Like Grant pages are marked by the grant driver using this flag. And
Xen page tables are using it in PV-guests for indicating a "Pinned"
page table.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread