From owner-freebsd-hackers@FreeBSD.ORG Mon Sep 27 21:13:05 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9ABC4106564A for ; Mon, 27 Sep 2010 21:13:05 +0000 (UTC) (envelope-from neelnatu@gmail.com) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id 0289F8FC14 for ; Mon, 27 Sep 2010 21:13:04 +0000 (UTC) Received: by wwc33 with SMTP id 33so395548wwc.31 for ; Mon, 27 Sep 2010 14:13:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=S85UfBFw7cHzEv+9Q2zdc1V839tui+ltoZelPnNmL/w=; b=qwf0u6dIAvysBkDHQsyR/TbJv/ZWAyVEObKJzK6BNxfXdMYH0uCcCPkyWVYwdcQyKO iNFwX9XBtqg/zHBDLh9uG6mtU8G0VA8Zr/7btxnSigCs3g8v0f05zs7pijxP7Z2jLGEC xa4cuxayawDFht6tYZL8gYv6MiJjx25m9ujQ8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=e2HK6escS8qbDCKR5Gu5n8ZZbZgVEvNA7SH9nG4RtyZ2dGB4NtSx60ps2lhrJAdJPN c6ZD3WLjRZlVSBbuJRJo2v4Zjn1N13zqsIgE6viD3Ldg9LNtvQ1HJjhJmrWR1oHXsR1N z6vWmBQfzFBh/2SEz7pUHNy/ZIsT5c/QIQe4A= MIME-Version: 1.0 Received: by 10.216.186.207 with SMTP id w57mr159664wem.19.1285621983690; Mon, 27 Sep 2010 14:13:03 -0700 (PDT) Received: by 10.216.133.5 with HTTP; Mon, 27 Sep 2010 14:13:03 -0700 (PDT) In-Reply-To: <201009271104.17478.jhb@freebsd.org> References: <201009271104.17478.jhb@freebsd.org> Date: Mon, 27 Sep 2010 14:13:03 -0700 Message-ID: From: Neel Natu To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org Subject: Re: PATCH: fix bogus error message "bus_dmamem_alloc failed to align memory properly" X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Sep 2010 21:13:05 -0000 Hi John, Thanks for reviewing this. On Mon, Sep 27, 2010 at 8:04 AM, John Baldwin wrote: > On Friday, September 24, 2010 9:00:44 pm Neel Natu wrote: >> Hi, >> >> This patch fixes the bogus error message from bus_dmamem_alloc() about >> the buffer not being aligned properly. >> >> The problem is that the check is against a virtual address as opposed >> to the physical address. contigmalloc() makes guarantees about >> the alignment of physical addresses but not the virtual address >> mapping it. >> >> Any objections if I commit this patch? > > Hmmm, I guess you are doing super-page alignment rather than sub-page > alignment? =A0In general I thought the busdma code only handled sub-page > alignment and doesn't fully handle requests for super-page alignment. > Yes, this is for allocations with sizes greater than PAGE_SIZE and alignment requirements also greater than a PAGE_SIZE. > For example, since it insists on walking individual pages at a time, if y= ou > had an alignment setting of 4 pages and passed in a single, aligned 4-pag= e > buffer, bus_dma would actually bounce the last 3 pages so that each indiv= idual > page is 4-page aligned. =A0At least, I think that is what would happen. > I think you are referring to bus_dmamap_load() operation that would follow the bus_dmamem_alloc(), right? The memory allocated by bus_dmamem_alloc() does not need to be bounced. In fact, the dmamap pointer returned by bus_dmamem_alloc() is NULL. At least for the amd64 implementation there is code in _bus_dmamap_load_buffer() which will coalesce individual dma segments if they satisfy 'boundary' and 'segsize' constraints. 683 /* 684 * Insert chunk into a segment, coalescing with 685 * previous segment if possible. 686 */ 687 if (first) { 688 segs[seg].ds_addr =3D curaddr; 689 segs[seg].ds_len =3D sgsize; 690 first =3D 0; 691 } else { 692 if (curaddr =3D=3D lastaddr && 693 (segs[seg].ds_len + sgsize) <=3D dmat->maxsegsz && 694 (dmat->boundary =3D=3D 0 || 695 (segs[seg].ds_addr & bmask) =3D=3D (curaddr & bmask))) 696 segs[seg].ds_len +=3D sgsize; 697 else { 698 if (++seg >=3D dmat->nsegments) 699 break; 700 segs[seg].ds_addr =3D curaddr; 701 segs[seg].ds_len =3D sgsize; 702 } 703 } I do get the expected result after I call dma_dmamap_load() i.e. a single dma segment with the correct alignment and boundary settings. > For sub-page alignment, the virtual and physical address alignments shoul= d be > the same. > Yes. best Neel >> best >> Neel >> >> Index: sys/powerpc/powerpc/busdma_machdep.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/powerpc/powerpc/busdma_machdep.c =A0 =A0 =A0(revision 213113) >> +++ sys/powerpc/powerpc/busdma_machdep.c =A0 =A0 =A0(working copy) >> @@ -529,7 +529,7 @@ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x = error %d", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __func__, dmat, dmat->flags, ENOMEM)= ; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM); >> - =A0 =A0 } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) { >> + =A0 =A0 } else if (vtophys(*vaddr) & (dmat->alignment - 1)) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem= ory properly.\n"); >> =A0 =A0 =A0 } >> =A0#ifdef NOTYET >> Index: sys/sparc64/sparc64/bus_machdep.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/sparc64/sparc64/bus_machdep.c (revision 213113) >> +++ sys/sparc64/sparc64/bus_machdep.c (working copy) >> @@ -652,7 +652,7 @@ >> =A0 =A0 =A0 } >> =A0 =A0 =A0 if (*vaddr =3D=3D NULL) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM); >> - =A0 =A0 if ((uintptr_t)*vaddr % dmat->dt_alignment) >> + =A0 =A0 if (vtophys(*vaddr) % dmat->dt_alignment) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("%s: failed to align memory properly.= \n", __func__); >> =A0 =A0 =A0 return (0); >> =A0} >> Index: sys/ia64/ia64/busdma_machdep.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/ia64/ia64/busdma_machdep.c =A0 =A0(revision 213113) >> +++ sys/ia64/ia64/busdma_machdep.c =A0 =A0(working copy) >> @@ -455,7 +455,7 @@ >> =A0 =A0 =A0 } >> =A0 =A0 =A0 if (*vaddr =3D=3D NULL) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM); >> - =A0 =A0 else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) >> + =A0 =A0 else if (vtophys(*vaddr) & (dmat->alignment - 1)) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem= ory properly.\n"); >> =A0 =A0 =A0 return (0); >> =A0} >> Index: sys/i386/i386/busdma_machdep.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/i386/i386/busdma_machdep.c =A0 =A0(revision 213113) >> +++ sys/i386/i386/busdma_machdep.c =A0 =A0(working copy) >> @@ -540,7 +540,7 @@ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x = error %d", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __func__, dmat, dmat->flags, ENOMEM)= ; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM); >> - =A0 =A0 } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) { >> + =A0 =A0 } else if (vtophys(*vaddr) & (dmat->alignment - 1)) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem= ory properly.\n"); >> =A0 =A0 =A0 } >> =A0 =A0 =A0 if (flags & BUS_DMA_NOCACHE) >> Index: sys/amd64/amd64/busdma_machdep.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/amd64/amd64/busdma_machdep.c =A0(revision 213113) >> +++ sys/amd64/amd64/busdma_machdep.c =A0(working copy) >> @@ -526,7 +526,7 @@ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x = error %d", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __func__, dmat, dmat->flags, ENOMEM)= ; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM); >> - =A0 =A0 } else if ((uintptr_t)*vaddr & (dmat->alignment - 1)) { >> + =A0 =A0 } else if (vtophys(*vaddr) & (dmat->alignment - 1)) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("bus_dmamem_alloc failed to align mem= ory properly.\n"); >> =A0 =A0 =A0 } >> =A0 =A0 =A0 if (flags & BUS_DMA_NOCACHE) >> _______________________________________________ >> freebsd-hackers@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers >> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.or= g" >> > > -- > John Baldwin >