From owner-freebsd-current@FreeBSD.ORG Thu Jul 12 16:37:20 2012 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 70ED2106566B for ; Thu, 12 Jul 2012 16:37:20 +0000 (UTC) (envelope-from scott4long@yahoo.com) Received: from nm27.bullet.mail.sp2.yahoo.com (nm27.bullet.mail.sp2.yahoo.com [98.139.91.97]) by mx1.freebsd.org (Postfix) with SMTP id 43F378FC1F for ; Thu, 12 Jul 2012 16:37:20 +0000 (UTC) Received: from [98.139.91.63] by nm27.bullet.mail.sp2.yahoo.com with NNFMP; 12 Jul 2012 16:37:14 -0000 Received: from [98.139.91.12] by tm3.bullet.mail.sp2.yahoo.com with NNFMP; 12 Jul 2012 16:37:14 -0000 Received: from [127.0.0.1] by omp1012.mail.sp2.yahoo.com with NNFMP; 12 Jul 2012 16:37:14 -0000 X-Yahoo-Newman-Property: ymail-3 X-Yahoo-Newman-Id: 230355.65709.bm@omp1012.mail.sp2.yahoo.com Received: (qmail 17167 invoked by uid 60001); 12 Jul 2012 16:37:14 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s1024; t=1342111033; bh=SWGQOvvnlC4DMGqAOEytp9L27viTQHQqeUJYAj1PHak=; h=X-YMail-OSG:Received:X-Mailer:References:Message-ID:Date:From:Reply-To:Subject:To:Cc:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding; b=4RCpwAkbajQPAPACSevSeonYFPLpW2jAlAGwj9WjJNH+MVymXhQ5dMve1VaHkp250VyJ7iQrTaudVyOontncWyty60+tsqoi/lojR2t+1jEfzC/o5QlNAM/SwU9Aj/hfAax8o5O6dOnPPHCpGDatITP60bBwV2zkiqBqeY4OBEM= DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=yahoo.com; h=X-YMail-OSG:Received:X-Mailer:References:Message-ID:Date:From:Reply-To:Subject:To:Cc:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding; b=DyPOJZuwa7E59cHIpReIvU8Lpl7e70X1FEaM9t1rIS3fJc6hWlYorbwzNml4bfJFU6vHe1hC4XTVHuU7Tfozs9Rk82Yu9Y0Nj2XDF2fCndufwFB/p6eIenK8tmKZKOo1OOlZWW9QhvOwq1katpJfrxELzReZB4AHqHt4mpwWXM4=; X-YMail-OSG: kYfG7pQVM1kZ_v4uPxzSB0GChF5GV5RRbbznV8p4LJsW6.a dUQXX7i0MrbOnzVUpjaXK5v1X_SwSmNHXqijuO8qWxw2g_UOs3NlgR0LKUgS kNVpeqNhDWdzLjgDG7RGGEhka_XlDmRwlQ2e3ErS7XXQOxdxSX64.r4vlUDT gT3pCO2bnz54IUGvb7kHiuy5.nBvMq3.BvbyTPOeLMdVTCYUgbmNsAq_gUKR D0DY.GDMYVEKUAx2YzNMdLIuAXKTyemrE3J4fQE1Q1K6wX5o19NkKdICoeUa Iu.WSP11rEru8H.g.bE.j120m0kaqERSnILyOwN2Onmz9npEJiskE_dakwPh 0uYtz55XxdSls3G662M8mJlmeUlksVbOzKnwJHDVXSbtdtLcpK0GY0rl350k NAIstjrBVjMnpERojSURXLOZAZVC0l8Amz3TY3jq3Kiz0y7CwfH213fw6RL_ SkTvboWQ5NmQ- Received: from [69.53.237.66] by web45716.mail.sp1.yahoo.com via HTTP; Thu, 12 Jul 2012 09:37:13 PDT X-Mailer: YahooMailWebService/0.8.120.356233 References: <201207121040.27116.jhb@freebsd.org> Message-ID: <1342111033.198.YahooMailNeo@web45716.mail.sp1.yahoo.com> Date: Thu, 12 Jul 2012 09:37:13 -0700 (PDT) From: Scott Long To: John Baldwin , "current@freebsd.org" In-Reply-To: <201207121040.27116.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Thu, 12 Jul 2012 16:40:00 +0000 Cc: Peter Jeremy Subject: Re: Adding support for WC (write-combining) memory to bus_dma X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Scott Long List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jul 2012 16:37:20 -0000 =0A=0A=0A=0A----- Original Message -----=0A> From: John Baldwin =0A> To: current@freebsd.org=0A> Cc: scottl@freebsd.org; Peter Jeremy= =0A> Sent: Thursday, July 12, 2012 7:40 AM=0A> Subject= : Adding support for WC (write-combining) memory to bus_dma=0A> =0A> I have= a need to allocate static DMA memory via bus_dmamem_alloc() that is =0A> a= lso WC (for a PCI-e device so it can use "nosnoop" transactions).=A0 =0A> T= his is =0A> similar to what the nvidia driver needs, but in my case it is m= uch cleaner to =0A> allocate the memory via bus dma since the existing code= I am extending all =0A> uses busdma.=0A> =0A> I have a patch to implement = this on 8.x for amd64 that I can port to HEAD if =0A> folks don't object.= =A0 What I would really like to do is add a new paramter to =0A> =0A> bus_d= mamem_alloc() to specify the memory attribute to use, but I am hesitant =0A= > to break that API.=A0 Instead, I added a new flag similar to the existing= =0A> BUS_DMA_NOCACHE used to allocate UC memory.=0A>=A0=0A=0APlease don't = add new parameters. =A0Now that I'm carefully documenting the=0Aevolution o= f the=A0APIs, it's becoming glaringly apparent how sloppy we are=0Awith API= design and=A0interface compatibility. =A0I'm just as guilty of it as anyon= e,=0Abut I'd really like to=A0see less instances of call signature changes = in existing=0Afunctions; they make=A0driver maintenance tedious and are har= d to effectively=0Adocument. =A0Some options I can think of:=0A=0A1. =A0cre= ate bus_dmamem_alloc_attr(). =A0I don't really like leafy API growth like= =0Athis either,=A0but=A0it's not a horrible solution.=0A2. =A0There are exi= sting placeholder flags, BUS_DMA_BUS[1234] that could be=0Aaliased and repu= rposed to hold 4 bits of attribute information for this function.=0AThe 3 a= nd 4 variants are already in use, but I haven't looked closely to see=0Athe= ir scope.=0A3. =A0Reserve the top 16 bits of the flags for attribute inform= ation.=0A4. =A0Move the attribute information into the tag and create new s= etter/getter=0Aaccessors for attribute information. =A0This would probably = be the cleanest,=0Athough it breaks the existing sloppiness of allowing dif= ferent pseudo-attributes=0Afor different allocations under the same tag. = =A0I've wanted to break down the=0Aexisting bus_dma_tag_create() into finer= -grained setter/getters for a while in=0Aany case.=0A5. =A0Move the attribu= te information into the map and force everyone to start=0Acreating maps for= static memory allocations. =A0This would actually add some=0Amissing unifo= rmity to the API and might actually be cleaner that option 4.=0A=0A> While = doing this, I ran into an old bug, which is that if you were to call =0A> b= us_dmamem_alloc() with BUS_DMA_NOCACHE but a tag that otherwise fell throug= h =0A> to using malloc() instead of contigmalloc(), bus_dmamem_alloc() woul= d actually=0A> change the state of the entire page.=A0 This seems wrong.=A0= Instead, I think that =0A> any request for a non-default memory attribute = should always use =0A> contigmalloc().=A0 In fact, even better is to call k= mem_alloc_contig() directly=0A> rather than using contigmalloc().=A0 Howeve= r, if you change this, then =0A> bus_dmamem_free() won't always DTRT as it = doesn't have enough state to =0A> know if=0A> a small allocation should be = free'd via free() or contigfree() (the latter =0A> would be required if it = used a non-default memory attribute).=A0 The fix I used =0A> for this was t= o create a new dummy dmamap that is returned by bus_dmamem_alloc =0A> if it= uses contigmalloc().=A0 bus_dmamem_free() then checks the passed in map = =0A> pointer to decide which type of free to perform.=A0 Once this is fixed= , the =0A> actual WC support is rather trivial as it merely consists of pas= sing a =0A> different argument to kmem_alloc_contig().=0A=0AYup, this is a = problem, and I like your fix; this kind of state is exactly what=0Abelongs = in the map.=0A=0A> =0A> Oh, and using kmem_alloc_contig() instead of the pm= ap_change_attr() hack is=0A> required if you want to be able to export the = same pages to userland via=0A> mmap (e.g. using an OBJT_SG object). :)=0A> = =0A> Peter, this is somewhat orthognal (but related) to your bus_dma patch = which is=0A> what prompted me to post this.=0A> =0A> Patch for 8 is below.= =A0 Porting it to HEAD should be fairly trivial and direct.=0A> =0A> Index:= amd64/amd64/busdma_machdep.c=0A=0AThanks,=0AScott=0A