freebsd stable - Sep 2006 - Patch: sym(4) "VTOBUS FAILED" panics on amd64, amd64/89550

Hi,

I posted this to freebsd-amd64, but I've had no replies.

This fixes the sym "VTOBUS FAILED" panics that I got consistently on
6.1-RELEASE/amd64;  see the PR amd64/89550 for dmesg, other reports, etc.

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.

I'm looking for someone to take a quick look at it and make sure that the
approach is reasonable.  I would be nice if a fix was committed (not
necessarily mine).

sym(4) is listed as supported in the 6.1/amd64 hardware guide, but it
doesn't seem to be working at all.  For 6.2, I either suggest putting in a
fix so that it is supported, or having a note in the hardware guide.

Sorry about coming along so late in the release cycle, but I first
encountered the problem on Monday.

Thanks,

Jan Mikkelsen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sym_hipd.diff
Type: application/octet-stream
Size: 4672 bytes
Desc: not available
Url :
http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20060921/edcadeab/sym_hipd.obj

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