Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 21 Sep 2006 19:17:33 -0700 (PDT)
From:      Doug White <dwhite@gumbysoft.com>
To:        Jan Mikkelsen <janm@transactionware.com>
Cc:        freebsd-stable@freebsd.org
Subject:   Re: Patch: sym(4) "VTOBUS FAILED" panics on amd64, amd64/89550
Message-ID:  <20060921190148.S87814@carver.gumbysoft.com>
In-Reply-To: <002d01c6ddc7$76d10cc0$0202a8c0@transactzbkv04>
References:  <002d01c6ddc7$76d10cc0$0202a8c0@transactzbkv04>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 22 Sep 2006, Jan Mikkelsen wrote:

> Quick summary:  sym(4) assumes on amd64 that virtual addresses provided by
> bus_dmamem_alloc() have the same alignment as the physical addresses (in
> this case, 2*PAGE_SIZE).  They don't, and stuff breaks.  This patch works
> around that.

Why is this? busdma supports alignment constraints; why not just set the 
alignment to what you need it set at? I realize sym has its own hand 
rolled DMA management craziness but alignment is something busdma can take 
care of easily.

Also the changes to MEMO_CLUSTER_SIZE seems to be already compensated for 
by the code blocks that calculate it:

#define MEMO_SHIFT      4       /* 16 bytes minimum memory chunk */
#ifndef __amd64__
#define MEMO_PAGE_ORDER 0       /* 1 PAGE  maximum */
#else
#define MEMO_PAGE_ORDER 1       /* 2 PAGEs maximum on amd64 */
#endif
#if 0
#define MEMO_FREE_UNUSED        /* Free unused pages immediately */
#endif
#define MEMO_WARN       1
#define MEMO_CLUSTER_SHIFT      (PAGE_SHIFT+MEMO_PAGE_ORDER)
#define MEMO_CLUSTER_SIZE       (1UL << MEMO_CLUSTER_SHIFT)
#define MEMO_CLUSTER_MASK       (MEMO_CLUSTER_SIZE-1)

This results in 2*PAGE_SIZE clusters on amd64 and PAGE_SIZE clusters on 
other platforms. Since you seem to like doing MEMO_CLUSTER_SIZE * 2, why 
not just increase MEMO_PAGE_ORDER to 2 (and get 4*PAGE_SIZE clusters) ?

Oh dear, I didn't notice that the call to bus_dma_tag_create() has bad 
arguments. From the (RELENG_6) source:

                 if (!bus_dma_tag_create(dev_dmat, 1, MEMO_CLUSTER_SIZE,
                                BUS_SPACE_MAXADDR_32BIT,
                                BUS_SPACE_MAXADDR_32BIT,
                                NULL, NULL, MEMO_CLUSTER_SIZE, 1,
                                MEMO_CLUSTER_SIZE, 0,
                                busdma_lock_mutex, &Giant, &mp->dmat)) {

As you fixed, that second BUS_SPACE_MAXADDR_32BIT should be 
BUS_SPACE_MAXADDR since its an exclusion zone. I'm suprised that doesn't 
fix it right there.

If increasing MEMO_PAGE_ORDER fixes the issue, then the #ifs are 
extraneous.

Also we generally prefer diffs in unidiff (-u) format, its a little easier 
to figure out exactly what changed just by looking at the diff. Thanks!

-- 
Doug White                    |  FreeBSD: The Power to Serve
dwhite@gumbysoft.com          |  www.FreeBSD.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060921190148.S87814>