Date: Fri, 22 Sep 2006 14:25:32 +1000 From: "Jan Mikkelsen" <janm@transactionware.com> To: "'Doug White'" <dwhite@gumbysoft.com> Cc: freebsd-stable@freebsd.org Subject: RE: Patch: sym(4) "VTOBUS FAILED" panics on amd64, amd64/89550 Message-ID: <003001c6ddff$278d52d0$d435083d@transactionware.com> In-Reply-To: <20060921190148.S87814@carver.gumbysoft.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
Doug White wrote:
> On Fri, 22 Sep 2006, Jan Mikkelsen wrote:
>=20
> > Quick summary: sym(4) assumes on amd64 that virtual=20
> addresses provided by
> > bus_dmamem_alloc() have the same alignment as the physical=20
> addresses (in
> > this case, 2*PAGE_SIZE). They don't, and stuff breaks. =20
> This patch works
> > around that.
>=20
> Why is this? busdma supports alignment constraints; why not=20
> just set the=20
> alignment to what you need it set at? I realize sym has its own hand=20
> rolled DMA management craziness but alignment is something=20
> busdma can take=20
> care of easily.
sym has the alignment requirement on the virtual address because of the
buddy memory allocation algorithm; changing how sym allocates memory
internally would remove the requirement. The buddy algorithm with 2^13
bytes aligned on a 2^12 byte (but not a 2^13 byte) boundary can provide =
two
chunks of 2^12 bytes but nothing greater than 2^12 bytes.
The VTOBUS failure is caused by the buddy implementation making =
alignment
assumptions which aren't true, and then getting the virtual addresses =
wrong.
Perhaps I'm just doing something wrong with bus_dma. I believe I set =
the
alignment requirements to be 2*PAGE_SIZE, and this is what I see for the
physical address. However the virtual address seems to only be page
aligned.
I can't see any mention of virtual address alignment in the bus_dma man
page. Can it take care of virtual address alignment? If so, how?
=20
> Also the changes to MEMO_CLUSTER_SIZE seems to be already=20
> compensated for=20
> by the code blocks that calculate it:
>=20
> #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)
>=20
> This results in 2*PAGE_SIZE clusters on amd64 and PAGE_SIZE=20
> clusters on=20
> other platforms. Since you seem to like doing=20
> MEMO_CLUSTER_SIZE * 2, why=20
> not just increase MEMO_PAGE_ORDER to 2 (and get 4*PAGE_SIZE=20
> clusters) ?
My fix (which I don't claim is optimal) is to allocate =
2*MEMO_CLUSTER_SIZE
bytes, and then use the point within that memory with the right =
alignment.
The alignment is not a hardware requirement; it is a bit manipulation
requirement because of the existing allocation code.
The cost is some extra allocated pages in return for a working system.
> Oh dear, I didn't notice that the call to=20
> bus_dma_tag_create() has bad=20
> arguments. From the (RELENG_6) source:
>=20
> if (!bus_dma_tag_create(dev_dmat, 1,=20
> 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,=20
> &mp->dmat)) {
>=20
> As you fixed, that second BUS_SPACE_MAXADDR_32BIT should be=20
> BUS_SPACE_MAXADDR since its an exclusion zone. I'm suprised=20
> that doesn't=20
> fix it right there.
I tried it, didn't fix it.
> Also we generally prefer diffs in unidiff (-u) format, its a=20
> little easier=20
> to figure out exactly what changed just by looking at the=20
> diff. Thanks!
Thanks, I'll do that next time.
Regards,
Jan Mikkelsen.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?003001c6ddff$278d52d0$d435083d>