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
[EMAIL PROTECTED]          |  www.FreeBSD.org
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to