Re: [Xen-devel] [PATCH v1] xen/swiotlb: rework early repeat code

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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 <sergey.dyasli@xxxxxxxxxx>

Just FYI I am touching the same code to fix another unrelated bug, see:

https://marc.info/?l=xen-devel&m=155856767022893


> ---
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> ---
>  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, &dma_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,
> +                            &contig_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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel