Patch: sym(4) "VTOBUS FAILED" panics on amd64, amd64/89550

[Jan Mikkelsen]

Hi,

Doug White wrote:
> 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.

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?
 
> 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) ?

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 
> 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.

I tried it, didn't fix it.

> 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!

Thanks, I'll do that next time.

Regards,

Jan Mikkelsen.