[PATCH RFC] Add size alignment to page allocator.

[PATCH RFC] Add size alignment to page allocator.

[Konrad Rzeszutek Wilk]

Hey!

Please see the two RFC patches to add in the XENMEM_exchange hypercall
another argument to get MFNs which are size aligned.

The use case is also attached - which is to provide the framework
for dma_alloc_coherent which sets certain guarantees that were not
provided by Linux Xen-SWIOTLB leading to driver corruptions.

Thanks!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH RFC 1/2] xen/page_alloc: Add size_align parameter to provide MFNs which are size aligned.

[Konrad Rzeszutek Wilk]

This is to support the requirement that exists in PV dom0
when doing DMA requests:

"dma_alloc_coherent()
[...]
The CPU virtual address and the DMA address are both guaranteed to be
aligned to the smallest PAGE_SIZE order which is greater than or equal
to the requested size.  This invariant exists (for example) to guarantee
that if you allocate a chunk which is smaller than or equal to 64
kilobytes, the extent of the buffer you receive will not cross a 64K
boundary."

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/common/memory.c         |  3 +++
 xen/common/page_alloc.c     | 22 +++++++++++++++++++++-
 xen/include/public/memory.h |  2 ++
 xen/include/xen/mm.h        |  2 ++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 21797ca..a4c0c54 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -475,6 +475,9 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
         (BITS_PER_LONG+PAGE_SHIFT)));
     memflags |= MEMF_node(XENMEMF_get_node(exch.out.mem_flags));
 
+    if ( XENMEMF_align_size & exch.out.mem_flags && is_hardware_domain(d) )
+        memflags |= MEMF_size_align;
+
     for ( i = (exch.nr_exchanged >> in_chunk_order);
           i < (exch.in.nr_extents >> in_chunk_order);
           i++ )
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ae2476d..e43f52f 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -738,7 +738,7 @@ static struct page_info *alloc_heap_pages(
      * Others try tmem pools then fail.  This is a workaround until all
      * post-dom0-creation-multi-page allocations can be eliminated.
      */
-    if ( ((order == 0) || (order >= 9)) &&
+    if ( ((order == 0) || (order >= 9)) && !(memflags & MEMF_size_align) &&
          (total_avail_pages <= midsize_alloc_zone_pages) &&
          tmem_freeable_pages() )
         goto try_tmem;
@@ -752,14 +752,34 @@ static struct page_info *alloc_heap_pages(
     {
         zone = zone_hi;
         do {
+            struct page_info *old = NULL;
+
             /* Check if target node can support the allocation. */
             if ( !avail[node] || (avail[node][zone] < request) )
                 continue;
 
             /* Find smallest order which can satisfy the request. */
             for ( j = order; j <= MAX_ORDER; j++ )
+            {
+ next_page:
                 if ( (pg = page_list_remove_head(&heap(node, zone, j))) )
+                {
+                    if ( memflags & MEMF_size_align )
+                    {
+                        if (pg == old)
+                            continue;
+
+                        if ( (page_to_mfn(pg) % request ) == 0 )
+                            goto found;
+
+                        page_list_add_tail(pg, &heap(node, zone, j));
+                        old = pg;
+                        pg = NULL;
+                        goto next_page;
+                    }
                     goto found;
+                }
+            }
         } while ( zone-- > zone_lo ); /* careful: unsigned zone may wrap */
 
         if ( (memflags & MEMF_exact_node) && req_node != NUMA_NO_NODE )
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 5bf840f..311e7d8 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -58,6 +58,8 @@
 #define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
 /* Flag to indicate the node specified is virtual node */
 #define XENMEMF_vnode  (1<<18)
+/* Flag to indicate the allocation to be size aligned. */
+#define XENMEMF_align_size (1U<<19)
 #endif
 
 struct xen_memory_reservation {
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 76fbb82..c505170 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -224,6 +224,8 @@ struct npfec {
 #define  MEMF_no_owner    (1U<<_MEMF_no_owner)
 #define _MEMF_no_tlbflush 6
 #define  MEMF_no_tlbflush (1U<<_MEMF_no_tlbflush)
+#define _MEMF_size_align  7
+#define  MEMF_size_align  (1U<<_MEMF_size_align)
 #define _MEMF_node        8
 #define  MEMF_node_mask   ((1U << (8 * sizeof(nodeid_t))) - 1)
 #define  MEMF_node(n)     ((((n) + 1) & MEMF_node_mask) << _MEMF_node)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH RFC 2/2] xen-swiotlb: Provide size aligned DMA addresses.

[Konrad Rzeszutek Wilk]

The dma_alloc_coherent() API specifies that:

[...]
The CPU virtual address and the DMA address are both guaranteed to be
aligned to the smallest PAGE_SIZE order which is greater than or equal
to the requested size.  This invariant exists (for example) to guarantee
that if you allocate a chunk which is smaller than or equal to 64
kilobytes, the extent of the buffer you receive will not cross a 64K
boundary."

This change, along with the XENMEMF_align_size in the hypervisor
allows us to provide DMA buffers that are size aligned.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/mmu.c             | 12 +++++++-----
 drivers/xen/swiotlb-xen.c      |  4 ++--
 include/xen/interface/memory.h | 10 ++++++++++
 include/xen/xen-ops.h          |  2 +-
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 7d5afdb..07e5a97 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2577,7 +2577,8 @@ static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
 			       unsigned long extents_out,
 			       unsigned int order_out,
 			       unsigned long *mfns_out,
-			       unsigned int address_bits)
+			       unsigned int address_bits,
+			       bool size_align)
 {
 	long rc;
 	int success;
@@ -2599,7 +2600,8 @@ static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
 	};
 
 	BUG_ON(extents_in << order_in != extents_out << order_out);
-
+        if (size_align)
+		exchange.out.address_bits |= XENMEMF_align_size;
 	rc = HYPERVISOR_memory_op(XENMEM_exchange, &exchange);
 	success = (exchange.nr_exchanged == extents_in);
 
@@ -2611,7 +2613,7 @@ static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
 
 int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
 				 unsigned int address_bits,
-				 dma_addr_t *dma_handle)
+				 dma_addr_t *dma_handle, bool size_align)
 {
 	unsigned long *in_frames = discontig_frames, out_frame;
 	unsigned long  flags;
@@ -2641,7 +2643,7 @@ int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
 	out_frame = virt_to_pfn(vstart);
 	success = xen_exchange_memory(1UL << order, 0, in_frames,
 				      1, order, &out_frame,
-				      address_bits);
+				      address_bits, size_align);
 
 	/* 3. Map the new extent in place of old pages. */
 	if (success)
@@ -2682,7 +2684,7 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
 
 	/* 3. Do the exchange for non-contiguous MFNs. */
 	success = xen_exchange_memory(1, order, &in_frame, 1UL << order,
-					0, out_frames, 0);
+					0, out_frames, 0, false);
 
 	/* 4. Map new pages in place of old pages. */
 	if (success)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 87e6035..4996685 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -173,7 +173,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
 			rc = xen_create_contiguous_region(
 				p + (i << IO_TLB_SHIFT),
 				get_order(slabs << IO_TLB_SHIFT),
-				dma_bits, &dma_handle);
+				dma_bits, &dma_handle, false);
 		} while (rc && dma_bits++ < max_dma_bits);
 		if (rc)
 			return rc;
@@ -334,7 +334,7 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 		*dma_handle = dev_addr;
 	else {
 		if (xen_create_contiguous_region(phys, order,
-						 fls64(dma_mask), dma_handle) != 0) {
+						 fls64(dma_mask), dma_handle, true) != 0) {
 			xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
 			return NULL;
 		}
diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index 9aa8988..b98271f 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -19,6 +19,16 @@
 #define XENMEM_increase_reservation 0
 #define XENMEM_decrease_reservation 1
 #define XENMEM_populate_physmap     6
+/*
+ * Maximum # bits addressable by the user of the allocated region (e.g., I/O
+ * devices often have a 32-bit limitation even in 64-bit systems). If zero
+ * then the user has no addressing restriction. This field is not used by
+ * XENMEM_decrease_reservation.
+ */
+
+/* Flag to indicate the allocation to be size aligned. */
+#define XENMEMF_align_size (1U<<19)
+
 struct xen_memory_reservation {
 
     /*
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index b5486e6..9aa3ab8 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -36,7 +36,7 @@ int xen_setup_shutdown_event(void);
 extern unsigned long *xen_contiguous_bitmap;
 int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
 				unsigned int address_bits,
-				dma_addr_t *dma_handle);
+				dma_addr_t *dma_handle, bool size_align);
 
 void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order);
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC 1/2] xen/page_alloc: Add size_align parameter to provide MFNs which are size aligned.

[Jan Beulich]

>>> On 30.11.16 at 05:39, <konrad@kernel.org> wrote:
> This is to support the requirement that exists in PV dom0
> when doing DMA requests:
> 
> "dma_alloc_coherent()
> [...]
> The CPU virtual address and the DMA address are both guaranteed to be
> aligned to the smallest PAGE_SIZE order which is greater than or equal
> to the requested size.  This invariant exists (for example) to guarantee
> that if you allocate a chunk which is smaller than or equal to 64
> kilobytes, the extent of the buffer you receive will not cross a 64K
> boundary."

So I'm having trouble understanding what it is that actually needs
fixing / changing here: Any order-N allocation will be order-N-aligned
already. Is your caller perhaps simply not passing in a large enough
order? And changing alloc_heap_pages(), which guarantees the
requested alignment already anyway (after all it takes an order
input, not a size one), looks completely pointless regardless of what
extra requirements you may want to put on the exchange hypercall.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC 1/2] xen/page_alloc: Add size_align parameter to provide MFNs which are size aligned.

[Konrad Rzeszutek Wilk]

On Wed, Nov 30, 2016 at 02:30:41AM -0700, Jan Beulich wrote:
> >>> On 30.11.16 at 05:39, <konrad@kernel.org> wrote:
> > This is to support the requirement that exists in PV dom0
> > when doing DMA requests:
> > 
> > "dma_alloc_coherent()
> > [...]
> > The CPU virtual address and the DMA address are both guaranteed to be
> > aligned to the smallest PAGE_SIZE order which is greater than or equal
> > to the requested size.  This invariant exists (for example) to guarantee
> > that if you allocate a chunk which is smaller than or equal to 64
> > kilobytes, the extent of the buffer you receive will not cross a 64K
> > boundary."
> 
> So I'm having trouble understanding what it is that actually needs
> fixing / changing here: Any order-N allocation will be order-N-aligned
> already. Is your caller perhaps simply not passing in a large enough
> order? And changing alloc_heap_pages(), which guarantees the
> requested alignment already anyway (after all it takes an order
> input, not a size one), looks completely pointless regardless of what
> extra requirements you may want to put on the exchange hypercall.

The page_alloc.c code walks through different order pages. Which means
that if it can't find one within the requested order pages it will
go one up (and so on). Eventually that means you do get the requested
order pages, but they are not guaranteed to be order aligned (as they
may be order aligned to a higher value).

> 
> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC 1/2] xen/page_alloc: Add size_align parameter to provide MFNs which are size aligned.

[Jan Beulich]

>>> On 30.11.16 at 17:42, <konrad.wilk@oracle.com> wrote:
> On Wed, Nov 30, 2016 at 02:30:41AM -0700, Jan Beulich wrote:
>> >>> On 30.11.16 at 05:39, <konrad@kernel.org> wrote:
>> > This is to support the requirement that exists in PV dom0
>> > when doing DMA requests:
>> > 
>> > "dma_alloc_coherent()
>> > [...]
>> > The CPU virtual address and the DMA address are both guaranteed to be
>> > aligned to the smallest PAGE_SIZE order which is greater than or equal
>> > to the requested size.  This invariant exists (for example) to guarantee
>> > that if you allocate a chunk which is smaller than or equal to 64
>> > kilobytes, the extent of the buffer you receive will not cross a 64K
>> > boundary."
>> 
>> So I'm having trouble understanding what it is that actually needs
>> fixing / changing here: Any order-N allocation will be order-N-aligned
>> already. Is your caller perhaps simply not passing in a large enough
>> order? And changing alloc_heap_pages(), which guarantees the
>> requested alignment already anyway (after all it takes an order
>> input, not a size one), looks completely pointless regardless of what
>> extra requirements you may want to put on the exchange hypercall.
> 
> The page_alloc.c code walks through different order pages. Which means
> that if it can't find one within the requested order pages it will
> go one up (and so on). Eventually that means you do get the requested
> order pages, but they are not guaranteed to be order aligned (as they
> may be order aligned to a higher value).

But that's _better_ alignment than you asked for then.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel