Re: Buffers not reachable by PCI

[Konrad Rzeszutek Wilk <konrad@darnok.org>]

On Wed, Dec 14, 2011 at 06:42:07PM +0000, Kalev, Leonid wrote:
> On 12/14/2011 06:42 PM, Taylor, Neal E wrote:
> > The last quoted printout is calculated the same way as the test that fails which leads me to question the computation's validity.
> 
> The computation validity seems OK to me (the 'phys' address is, as correctly stated 
> in the comments, not suitable for DMA, but it is first translated via 
> xen_phys_to_bus, which gives the machine address - and on x86 that is also the bus 
> address)
> 
> I have a STUPID question, though: why are the start and end addresses of the swiotlb 
> memory area not page-aligned???

That is not a stupid question.
> 
> The io_tlb_end address is one byte PAST the valid area, which is why the 
> xen_swiotlb_dma_supported function uses (xen_io_tlb_end-1). However, this will work 
> properly only if the value is page-aligned. If it isn't, then decrementing it by one 
> will keep the value in the same page (which is one page past the last valid one).
> 
> The function that allocates the memory uses alloc_bootmem(), which provides just 
> cache-aligned memory (not page-aligned).

Yup.

It is actually funny (sad?), b/c I am the committer for the
e79f86b2ef9c0a8c47225217c1018b7d3d90101c which adds something like this:

 alloc_bootmem_pages(PAGE_ALIGN(io_tlb_nslabs * sizeof(int)

in the swiotlb code but I completly missed doing it for the Xen SWIOTLB.

<sigh>

I think this patch:

>From 47409eecc08effe20fc4aa0da899dd6ac475cb0b Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 14 Dec 2011 20:48:01 -0500
Subject: [PATCH] xen/swiotlb: Use page alignment for early buffer allocation.

This piggybacks on git commit e79f86b2ef9c0a8c47225217c1018b7d3d90101c
"swiotlb: Use page alignment for early buffer allocation" which:

"We could call free_bootmem_late() if swiotlb is not used, and
it will shrink to page alignment.

So alloc them with page alignment at first, to avoid lose two pages"

Reported-by: "Kalev, Leonid" <Leonid.Kalev@ca.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/swiotlb-xen.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 8e964b9..5c8e445 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -166,7 +166,8 @@ retry:
 	/*
 	 * Get IO TLB memory from any location.
 	 */
-	xen_io_tlb_start = alloc_bootmem(bytes);
+	xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
+
 	if (!xen_io_tlb_start) {
 		m = "Cannot allocate Xen-SWIOTLB buffer!\n";
 		goto error;
@@ -179,7 +180,7 @@ retry:
 			       bytes,
 			       xen_io_tlb_nslabs);
 	if (rc) {
-		free_bootmem(__pa(xen_io_tlb_start), bytes);
+		free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
 		m = "Failed to get contiguous memory for DMA from Xen!\n"\
 		    "You either: don't have the permissions, do not have"\
 		    " enough free memory under 4GB, or the hypervisor memory"\
-- 
1.7.1

is in order.
> 
> If it is OK for the swiotlb area not to be page-aligned, then the 
> xen_swiotlb_dma_supported should use (xen_io_tlb_end - (PAGE_SIZE-1))

Lets make it page aligned.

> 
> If the memory should be in fact aligned, then the allocation must be changed to 
> alloc_bootmem_pages() (which is the same as alloc_bootmem, but page-aligned).