Date: Sun, 1 Nov 2020 18:00:44 +0100 From: Marcin Wojtas <mw@semihalf.com> To: Andrew Turner <andrew@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Artur Rojek <ar@semihalf.com> Subject: Re: svn commit: r366106 - head/sys/arm64/arm64 Message-ID: <CAPv3WKe1NA48s8fjEsUJMRfG8udWDMiwN=zjuz-pxtrVrMmvxA@mail.gmail.com> In-Reply-To: <202009240717.08O7H55G090719@repo.freebsd.org> References: <202009240717.08O7H55G090719@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi, With this commit SDHCI fails to allocate a bounce buffer for SDMA (sdhci_dma_alloc() routine). The same behavior was observed on LS1046A and Armada 7k8k. Example log: sdhci_xenon0: <Armada Xenon SDHCI controller> mem 0x780000-0x7802ff irq 38 on simplebus3 getaddr: error 27 sdhci_xenon0-slot0: Can't load DMA memory for SDMA device_attach: sdhci_xenon0 attach returned 6 I debugged it a bit: * bus_dmamap_load returns EFBIG (error =3D 27) * The tag is created with an alignment to 128k (https://github.com/freebsd/freebsd/blob/master/sys/dev/sdhci/sdhci.c#L752) * When I set the alignment to anything =3D< PAGE_SIZE it works again: --- a/sys/dev/sdhci/sdhci.c +++ b/sys/dev/sdhci/sdhci.c @@ -749,7 +749,7 @@ sdhci_dma_alloc(struct sdhci_slot *slot) * forming the actual address of data, requiring the SDMA buffer to * be aligned to the SDMA boundary. */ - err =3D bus_dma_tag_create(bus_get_dma_tag(slot->bus), slot->sdma_b= bufsz, + err =3D bus_dma_tag_create(bus_get_dma_tag(slot->bus), PAGE_SIZE, 0, BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL, NULL, slot->sdma_bbufsz, 1, slot->sdma_bbufsz, BUS_DMA_ALLOCNOW, NULL, NULL, &slot->dmatag); I don't know if it's a valid fix though given a comment in code above (Linux however aligns DMA buffer to 512). Comments appreciated. Any reason why the huge alignment value worked before the r366106 and now it is problematic? Best regards, Marcin czw., 24 wrz 2020 o 09:17 Andrew Turner <andrew@freebsd.org> napisa=C5=82(a= ): > > Author: andrew > Date: Thu Sep 24 07:17:05 2020 > New Revision: 366106 > URL: https://svnweb.freebsd.org/changeset/base/366106 > > Log: > Bounce in more cases in the arm64 busdma > > We need to use a bounce buffer when the memory we are operating on is n= ot > aligned to a cacheline, and not aligned to the maps alignment. > > The former is to stop other threads from dirtying the cacheline while w= e > are performing DMA operations with it. The latter is to check memory > passed in by a driver is correctly aligned for the device. > > Reviewed by: mmel > Sponsored by: Innovate UK > Differential Revision: https://reviews.freebsd.org/D26496 > > Modified: > head/sys/arm64/arm64/busdma_bounce.c > > Modified: head/sys/arm64/arm64/busdma_bounce.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=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/arm64/arm64/busdma_bounce.c Thu Sep 24 07:13:13 2020 = (r366105) > +++ head/sys/arm64/arm64/busdma_bounce.c Thu Sep 24 07:17:05 2020 = (r366106) > @@ -139,6 +139,7 @@ struct bus_dmamap { > u_int flags; > #define DMAMAP_COHERENT (1 << 0) > #define DMAMAP_FROM_DMAMEM (1 << 1) > +#define DMAMAP_MBUF (1 << 2) > int sync_count; > struct sync_list slist[]; > }; > @@ -155,8 +156,8 @@ static bus_addr_t add_bounce_page(bus_dma_tag_t dmat, > vm_offset_t vaddr, bus_addr_t addr, bus_size_t size); > static void free_bounce_page(bus_dma_tag_t dmat, struct bounce_page *bpa= ge); > int run_filter(bus_dma_tag_t dmat, bus_addr_t paddr); > -static bool _bus_dmamap_pagesneeded(bus_dma_tag_t dmat, vm_paddr_t buf, > - bus_size_t buflen, int *pagesneeded); > +static bool _bus_dmamap_pagesneeded(bus_dma_tag_t dmat, bus_dmamap_t map= , > + vm_paddr_t buf, bus_size_t buflen, int *pagesneeded); > static void _bus_dmamap_count_pages(bus_dma_tag_t dmat, bus_dmamap_t map= , > pmap_t pmap, void *buf, bus_size_t buflen, int flags); > static void _bus_dmamap_count_phys(bus_dma_tag_t dmat, bus_dmamap_t map, > @@ -164,20 +165,70 @@ static void _bus_dmamap_count_phys(bus_dma_tag_t dm= at, > static int _bus_dmamap_reserve_pages(bus_dma_tag_t dmat, bus_dmamap_t ma= p, > int flags); > > +/* > + * Return true if the DMA should bounce because the start or end does no= t fall > + * on a cacheline boundary (which would require a partial cacheline flus= h). > + * COHERENT memory doesn't trigger cacheline flushes. Memory allocated = by > + * bus_dmamem_alloc() is always aligned to cacheline boundaries, and the= re's a > + * strict rule that such memory cannot be accessed by the CPU while DMA = is in > + * progress (or by multiple DMA engines at once), so that it's always sa= fe to do > + * full cacheline flushes even if that affects memory outside the range = of a > + * given DMA operation that doesn't involve the full allocated buffer. = If we're > + * mapping an mbuf, that follows the same rules as a buffer we allocated= . > + */ > static bool > -might_bounce(bus_dma_tag_t dmat) > +cacheline_bounce(bus_dma_tag_t dmat, bus_dmamap_t map, bus_addr_t paddr, > + bus_size_t size) > { > > +#define DMAMAP_CACHELINE_FLAGS = \ > + (DMAMAP_FROM_DMAMEM | DMAMAP_COHERENT | DMAMAP_MBUF) > + if ((dmat->bounce_flags & BF_COHERENT) !=3D 0) > + return (false); > + if (map !=3D NULL && (map->flags & DMAMAP_CACHELINE_FLAGS) !=3D 0= ) > + return (false); > + return (((paddr | size) & (dcache_line_size - 1)) !=3D 0); > +#undef DMAMAP_CACHELINE_FLAGS > +} > + > +/* > + * Return true if the given address does not fall on the alignment bound= ary. > + */ > +static bool > +alignment_bounce(bus_dma_tag_t dmat, bus_addr_t addr) > +{ > + > + return ((addr & (dmat->common.alignment - 1)) !=3D 0); > +} > + > +static bool > +might_bounce(bus_dma_tag_t dmat, bus_dmamap_t map, bus_addr_t paddr, > + bus_size_t size) > +{ > + > if ((dmat->bounce_flags & BF_COULD_BOUNCE) !=3D 0) > return (true); > > + if (cacheline_bounce(dmat, map, paddr, size)) > + return (true); > + > + if (alignment_bounce(dmat, paddr)) > + return (true); > + > return (false); > } > > static bool > -must_bounce(bus_dma_tag_t dmat, bus_addr_t paddr) > +must_bounce(bus_dma_tag_t dmat, bus_dmamap_t map, bus_addr_t paddr, > + bus_size_t size) > { > > + if (cacheline_bounce(dmat, map, paddr, size)) > + return (true); > + > + if (alignment_bounce(dmat, paddr)) > + return (true); > + > if ((dmat->bounce_flags & BF_COULD_BOUNCE) !=3D 0 && > bus_dma_run_filter(&dmat->common, paddr)) > return (true); > @@ -240,8 +291,7 @@ bounce_bus_dma_tag_create(bus_dma_tag_t parent, bus_s= i > newtag->common.alignment > 1) > newtag->bounce_flags |=3D BF_COULD_BOUNCE; > > - if (((newtag->bounce_flags & BF_COULD_BOUNCE) !=3D 0) && > - (flags & BUS_DMA_ALLOCNOW) !=3D 0) { > + if ((flags & BUS_DMA_ALLOCNOW) !=3D 0) { > struct bounce_zone *bz; > > /* Must bounce */ > @@ -315,9 +365,9 @@ static bool > bounce_bus_dma_id_mapped(bus_dma_tag_t dmat, vm_paddr_t buf, bus_size_t = buflen) > { > > - if (!might_bounce(dmat)) > + if (!might_bounce(dmat, NULL, buf, buflen)) > return (true); > - return (!_bus_dmamap_pagesneeded(dmat, buf, buflen, NULL)); > + return (!_bus_dmamap_pagesneeded(dmat, NULL, buf, buflen, NULL)); > } > > static bus_dmamap_t > @@ -373,43 +423,39 @@ bounce_bus_dmamap_create(bus_dma_tag_t dmat, int fl= ags > * exclusion region, a data alignment that is stricter than 1, an= d/or > * an active address boundary. > */ > - if (dmat->bounce_flags & BF_COULD_BOUNCE) { > - /* Must bounce */ > - if (dmat->bounce_zone =3D=3D NULL) { > - if ((error =3D alloc_bounce_zone(dmat)) !=3D 0) { > - free(*mapp, M_DEVBUF); > - return (error); > - } > + if (dmat->bounce_zone =3D=3D NULL) { > + if ((error =3D alloc_bounce_zone(dmat)) !=3D 0) { > + free(*mapp, M_DEVBUF); > + return (error); > } > - bz =3D dmat->bounce_zone; > + } > + bz =3D dmat->bounce_zone; > > - /* > - * Attempt to add pages to our pool on a per-instance > - * basis up to a sane limit. > - */ > - if (dmat->common.alignment > 1) > - maxpages =3D MAX_BPAGES; > - else > - maxpages =3D MIN(MAX_BPAGES, Maxmem - > - atop(dmat->common.lowaddr)); > - if ((dmat->bounce_flags & BF_MIN_ALLOC_COMP) =3D=3D 0 || > - (bz->map_count > 0 && bz->total_bpages < maxpages)) { > - pages =3D MAX(atop(dmat->common.maxsize), 1); > - pages =3D MIN(maxpages - bz->total_bpages, pages)= ; > - pages =3D MAX(pages, 1); > - if (alloc_bounce_pages(dmat, pages) < pages) > - error =3D ENOMEM; > - if ((dmat->bounce_flags & BF_MIN_ALLOC_COMP) > - =3D=3D 0) { > - if (error =3D=3D 0) { > - dmat->bounce_flags |=3D > - BF_MIN_ALLOC_COMP; > - } > - } else > - error =3D 0; > - } > - bz->map_count++; > + /* > + * Attempt to add pages to our pool on a per-instance > + * basis up to a sane limit. > + */ > + if (dmat->common.alignment > 1) > + maxpages =3D MAX_BPAGES; > + else > + maxpages =3D MIN(MAX_BPAGES, Maxmem - > + atop(dmat->common.lowaddr)); > + if ((dmat->bounce_flags & BF_MIN_ALLOC_COMP) =3D=3D 0 || > + (bz->map_count > 0 && bz->total_bpages < maxpages)) { > + pages =3D MAX(atop(dmat->common.maxsize), 1); > + pages =3D MIN(maxpages - bz->total_bpages, pages); > + pages =3D MAX(pages, 1); > + if (alloc_bounce_pages(dmat, pages) < pages) > + error =3D ENOMEM; > + if ((dmat->bounce_flags & BF_MIN_ALLOC_COMP) =3D=3D 0) { > + if (error =3D=3D 0) { > + dmat->bounce_flags |=3D BF_MIN_ALLOC_COMP= ; > + } > + } else > + error =3D 0; > } > + bz->map_count++; > + > if (error =3D=3D 0) { > dmat->map_count++; > if ((dmat->bounce_flags & BF_COHERENT) !=3D 0) > @@ -595,8 +641,8 @@ bounce_bus_dmamem_free(bus_dma_tag_t dmat, void *vadd= r > } > > static bool > -_bus_dmamap_pagesneeded(bus_dma_tag_t dmat, vm_paddr_t buf, bus_size_t b= uflen, > - int *pagesneeded) > +_bus_dmamap_pagesneeded(bus_dma_tag_t dmat, bus_dmamap_t map, vm_paddr_t= buf, > + bus_size_t buflen, int *pagesneeded) > { > bus_addr_t curaddr; > bus_size_t sgsize; > @@ -610,7 +656,7 @@ _bus_dmamap_pagesneeded(bus_dma_tag_t dmat, vm_paddr_= t > curaddr =3D buf; > while (buflen !=3D 0) { > sgsize =3D MIN(buflen, dmat->common.maxsegsz); > - if (must_bounce(dmat, curaddr)) { > + if (must_bounce(dmat, map, curaddr, sgsize)) { > sgsize =3D MIN(sgsize, > PAGE_SIZE - (curaddr & PAGE_MASK)); > if (pagesneeded =3D=3D NULL) > @@ -632,7 +678,8 @@ _bus_dmamap_count_phys(bus_dma_tag_t dmat, bus_dmamap= _ > { > > if (map->pagesneeded =3D=3D 0) { > - _bus_dmamap_pagesneeded(dmat, buf, buflen, &map->pagesnee= ded); > + _bus_dmamap_pagesneeded(dmat, map, buf, buflen, > + &map->pagesneeded); > CTR1(KTR_BUSDMA, "pagesneeded=3D %d\n", map->pagesneeded)= ; > } > } > @@ -666,7 +713,9 @@ _bus_dmamap_count_pages(bus_dma_tag_t dmat, bus_dmama= p > paddr =3D pmap_kextract(vaddr); > else > paddr =3D pmap_extract(pmap, vaddr); > - if (must_bounce(dmat, paddr)) { > + if (must_bounce(dmat, map, paddr, > + min(vendaddr - vaddr, (PAGE_SIZE - ((vm_offse= t_t)vaddr & > + PAGE_MASK)))) !=3D 0) { > sg_len =3D roundup2(sg_len, > dmat->common.alignment); > map->pagesneeded++; > @@ -764,7 +813,7 @@ bounce_bus_dmamap_load_phys(bus_dma_tag_t dmat, bus_d= m > if (segs =3D=3D NULL) > segs =3D dmat->segments; > > - if (might_bounce(dmat)) { > + if (might_bounce(dmat, map, (bus_addr_t)buf, buflen)) { > _bus_dmamap_count_phys(dmat, map, buf, buflen, flags); > if (map->pagesneeded !=3D 0) { > error =3D _bus_dmamap_reserve_pages(dmat, map, fl= ags); > @@ -779,7 +828,8 @@ bounce_bus_dmamap_load_phys(bus_dma_tag_t dmat, bus_d= m > while (buflen > 0) { > curaddr =3D buf; > sgsize =3D MIN(buflen, dmat->common.maxsegsz); > - if (map->pagesneeded !=3D 0 && must_bounce(dmat, curaddr)= ) { > + if (map->pagesneeded !=3D 0 && > + must_bounce(dmat, map, curaddr, sgsize)) { > sgsize =3D MIN(sgsize, PAGE_SIZE - (curaddr & PAG= E_MASK)); > curaddr =3D add_bounce_page(dmat, map, 0, curaddr= , > sgsize); > @@ -833,7 +883,10 @@ bounce_bus_dmamap_load_buffer(bus_dma_tag_t dmat, bu= s_ > if (segs =3D=3D NULL) > segs =3D dmat->segments; > > - if (might_bounce(dmat)) { > + if (flags & BUS_DMA_LOAD_MBUF) > + map->flags |=3D DMAMAP_MBUF; > + > + if (might_bounce(dmat, map, (bus_addr_t)buf, buflen)) { > _bus_dmamap_count_pages(dmat, map, pmap, buf, buflen, fla= gs); > if (map->pagesneeded !=3D 0) { > error =3D _bus_dmamap_reserve_pages(dmat, map, fl= ags); > @@ -864,7 +917,8 @@ bounce_bus_dmamap_load_buffer(bus_dma_tag_t dmat, bus= _ > */ > max_sgsize =3D MIN(buflen, dmat->common.maxsegsz); > sgsize =3D PAGE_SIZE - (curaddr & PAGE_MASK); > - if (map->pagesneeded !=3D 0 && must_bounce(dmat, curaddr)= ) { > + if (map->pagesneeded !=3D 0 && > + must_bounce(dmat, map, curaddr, sgsize)) { > sgsize =3D roundup2(sgsize, dmat->common.alignmen= t); > sgsize =3D MIN(sgsize, max_sgsize); > curaddr =3D add_bounce_page(dmat, map, kvaddr, cu= raddr, > @@ -949,6 +1003,7 @@ bounce_bus_dmamap_unload(bus_dma_tag_t dmat, bus_dma= ma > } > > map->sync_count =3D 0; > + map->flags &=3D ~DMAMAP_MBUF; > } > > static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPv3WKe1NA48s8fjEsUJMRfG8udWDMiwN=zjuz-pxtrVrMmvxA>