Re: [Xen-devel] [PATCH v1] xen/swiotlb: rework early repeat code
On Fri, 24 May 2019, Sergey Dyasli wrote:
> Current repeat code is plain broken for the early=true case. Xen exchanges
> all DMA (<4GB) pages that it can on the first xen_swiotlb_fixup() attempt.
> All further attempts with a halved region will fail immediately because
> all DMA pages already belong to Dom0.
>
> Introduce contig_pages param for xen_swiotlb_fixup() to track the number
> of pages that were made contiguous in MFN space and use the same bootmem
> region while halving the memory requirements.
>
> Signed-off-by: Sergey Dyasli
Just FYI I am touching the same code to fix another unrelated bug, see:
https://marc.info/?l=xen-devel=155856767022893
> ---
> CC: Konrad Rzeszutek Wilk
> CC: Boris Ostrovsky
> CC: Juergen Gross
> CC: Stefano Stabellini
> CC: Paul Durrant
> ---
> drivers/xen/swiotlb-xen.c | 36 ++--
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 5dcb06fe9667..d2aba804d06c 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -142,7 +142,8 @@ static int is_xen_swiotlb_buffer(dma_addr_t dma_addr)
> static int max_dma_bits = 32;
>
> static int
> -xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
> +xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs,
> + unsigned long *contig_pages)
> {
> int i, rc;
> int dma_bits;
> @@ -156,10 +157,13 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long
> nslabs)
> int slabs = min(nslabs - i, (unsigned long)IO_TLB_SEGSIZE);
>
> do {
> + unsigned int order = get_order(slabs << IO_TLB_SHIFT);
> rc = xen_create_contiguous_region(
> p + (i << IO_TLB_SHIFT),
> - get_order(slabs << IO_TLB_SHIFT),
> + order,
> dma_bits, _handle);
> + if (rc == 0)
> + *contig_pages += 1 << order;
> } while (rc && dma_bits++ < max_dma_bits);
> if (rc)
> return rc;
> @@ -202,7 +206,7 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err
> err)
> }
> int __ref xen_swiotlb_init(int verbose, bool early)
> {
> - unsigned long bytes, order;
> + unsigned long bytes, order, contig_pages;
> int rc = -ENOMEM;
> enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
> unsigned int repeat = 3;
> @@ -244,13 +248,32 @@ int __ref xen_swiotlb_init(int verbose, bool early)
> /*
>* And replace that memory with pages under 4GB.
>*/
> + contig_pages = 0;
> rc = xen_swiotlb_fixup(xen_io_tlb_start,
> bytes,
> -xen_io_tlb_nslabs);
> +xen_io_tlb_nslabs,
> +_pages);
> if (rc) {
> - if (early)
> + if (early) {
> + unsigned long orig_bytes = bytes;
> + while (repeat-- > 0) {
> + xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
> + (xen_io_tlb_nslabs >> 1));
> + pr_info("Lowering to %luMB\n",
> + (xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
> + bytes = xen_set_nslabs(xen_io_tlb_nslabs);
> + order = get_order(xen_io_tlb_nslabs <<
> IO_TLB_SHIFT);
> + xen_io_tlb_end = xen_io_tlb_start + bytes;
> + if (contig_pages >= (1ul << order)) {
> + /* Enough pages were made contiguous */
> + memblock_free(__pa(xen_io_tlb_start +
> bytes),
> + PAGE_ALIGN(orig_bytes -
> bytes));
> + goto fixup_done;
> + }
> + }
> memblock_free(__pa(xen_io_tlb_start),
> PAGE_ALIGN(bytes));
> + }
> else {
> free_pages((unsigned long)xen_io_tlb_start, order);
> xen_io_tlb_start = NULL;
> @@ -258,6 +281,7 @@ int __ref xen_swiotlb_init(int verbose, bool early)
> m_ret = XEN_SWIOTLB_EFIXUP;
> goto error;
> }
> +fixup_done:
> start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> if (early) {
> if (swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs,
> @@ -272,7 +296,7 @@ int __ref xen_swiotlb_init(int verbose, bool early)
>
> return rc;
> error:
> - if (repeat--) {
> + if (repeat-- > 0) {
> xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
>
[Xen-devel] [PATCH v1] xen/swiotlb: rework early repeat code
Current repeat code is plain broken for the early=true case. Xen exchanges
all DMA (<4GB) pages that it can on the first xen_swiotlb_fixup() attempt.
All further attempts with a halved region will fail immediately because
all DMA pages already belong to Dom0.
Introduce contig_pages param for xen_swiotlb_fixup() to track the number
of pages that were made contiguous in MFN space and use the same bootmem
region while halving the memory requirements.
Signed-off-by: Sergey Dyasli
---
CC: Konrad Rzeszutek Wilk
CC: Boris Ostrovsky
CC: Juergen Gross
CC: Stefano Stabellini
CC: Paul Durrant
---
drivers/xen/swiotlb-xen.c | 36 ++--
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 5dcb06fe9667..d2aba804d06c 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -142,7 +142,8 @@ static int is_xen_swiotlb_buffer(dma_addr_t dma_addr)
static int max_dma_bits = 32;
static int
-xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
+xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs,
+ unsigned long *contig_pages)
{
int i, rc;
int dma_bits;
@@ -156,10 +157,13 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long
nslabs)
int slabs = min(nslabs - i, (unsigned long)IO_TLB_SEGSIZE);
do {
+ unsigned int order = get_order(slabs << IO_TLB_SHIFT);
rc = xen_create_contiguous_region(
p + (i << IO_TLB_SHIFT),
- get_order(slabs << IO_TLB_SHIFT),
+ order,
dma_bits, _handle);
+ if (rc == 0)
+ *contig_pages += 1 << order;
} while (rc && dma_bits++ < max_dma_bits);
if (rc)
return rc;
@@ -202,7 +206,7 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err
err)
}
int __ref xen_swiotlb_init(int verbose, bool early)
{
- unsigned long bytes, order;
+ unsigned long bytes, order, contig_pages;
int rc = -ENOMEM;
enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
unsigned int repeat = 3;
@@ -244,13 +248,32 @@ int __ref xen_swiotlb_init(int verbose, bool early)
/*
* And replace that memory with pages under 4GB.
*/
+ contig_pages = 0;
rc = xen_swiotlb_fixup(xen_io_tlb_start,
bytes,
- xen_io_tlb_nslabs);
+ xen_io_tlb_nslabs,
+ _pages);
if (rc) {
- if (early)
+ if (early) {
+ unsigned long orig_bytes = bytes;
+ while (repeat-- > 0) {
+ xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
+ (xen_io_tlb_nslabs >> 1));
+ pr_info("Lowering to %luMB\n",
+(xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
+ bytes = xen_set_nslabs(xen_io_tlb_nslabs);
+ order = get_order(xen_io_tlb_nslabs <<
IO_TLB_SHIFT);
+ xen_io_tlb_end = xen_io_tlb_start + bytes;
+ if (contig_pages >= (1ul << order)) {
+ /* Enough pages were made contiguous */
+ memblock_free(__pa(xen_io_tlb_start +
bytes),
+PAGE_ALIGN(orig_bytes -
bytes));
+ goto fixup_done;
+ }
+ }
memblock_free(__pa(xen_io_tlb_start),
PAGE_ALIGN(bytes));
+ }
else {
free_pages((unsigned long)xen_io_tlb_start, order);
xen_io_tlb_start = NULL;
@@ -258,6 +281,7 @@ int __ref xen_swiotlb_init(int verbose, bool early)
m_ret = XEN_SWIOTLB_EFIXUP;
goto error;
}
+fixup_done:
start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
if (early) {
if (swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs,
@@ -272,7 +296,7 @@ int __ref xen_swiotlb_init(int verbose, bool early)
return rc;
error:
- if (repeat--) {
+ if (repeat-- > 0) {
xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
(xen_io_tlb_nslabs >> 1));
pr_info("Lowering to %luMB\n",
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
