Page MenuHomeFreeBSD
Differential D13918

Check the return value from utf8_to_ucs2 instead of whether or not uv is NULL. That's more correct and doesn't depend on the error behavior of utf8_to_ucs2. In practice, we'll never see this though since we pass utf8_to_ucs2 a well formed string.
ClosedPublic
Actions

Authored by imp on Jan 15 2018, 5:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 17, 3:30 AM
Unknown Object (File)
Mar 10 2025, 5:58 PM
Unknown Object (File)
Feb 17 2025, 3:57 AM
Unknown Object (File)
Feb 11 2025, 10:06 PM
Unknown Object (File)
Feb 8 2025, 4:58 PM
Unknown Object (File)
Feb 8 2025, 9:36 AM
Unknown Object (File)
Jan 25 2025, 7:28 PM
Unknown Object (File)
Jan 24 2025, 6:54 PM
View All 59 Files
Subscribers
None

Details

Reviewers
andrew
tsoome
Commits
rS328030: Check the return value from utf8_to_ucs2 instead of whether or not uv

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp created this revision.Jan 15 2018, 5:03 PM
Harbormaster completed remote builds in B14385: Diff 37976.Jan 15 2018, 5:03 PM
imp added reviewers: andrew, tsoome.Jan 15 2018, 5:03 PM
tsoome added inline comments.Jan 15 2018, 7:58 PM
stand/efi/boot1/boot1.c
105 ↗(On Diff #37976)

If we check the return value below, we do not need to set NULL here, except if the compiler will trigger may be used uninitialized warning...

imp added inline comments.Jan 15 2018, 10:09 PM
stand/efi/boot1/boot1.c
105 ↗(On Diff #37976)

We do, because we want utf8_to_ucs2 to allocate space.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 15 2018, 10:17 PM
Closed by commit rS328030: Check the return value from utf8_to_ucs2 instead of whether or not uv (authored by imp). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
stand/
efi/
boot1/
3 lines

Diff 38001

View Options

head/stand/efi/boot1/boot1.c

Show All 28 Lines
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE. * SUCH DAMAGE.
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
#include <sys/param.h> #include <sys/param.h>
#include <sys/systm.h> #include <sys/systm.h>
#include <sys/domainset.h>
#include <sys/malloc.h> #include <sys/malloc.h>
#include <sys/bus.h> #include <sys/bus.h>
#include <sys/interrupt.h> #include <sys/interrupt.h>
#include <sys/kernel.h> #include <sys/kernel.h>
#include <sys/ktr.h> #include <sys/ktr.h>
#include <sys/lock.h> #include <sys/lock.h>
#include <sys/proc.h> #include <sys/proc.h>
#include <sys/memdesc.h> #include <sys/memdesc.h>
▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Lines
static void _bus_dmamap_count_pages(bus_dma_tag_t dmat, bus_dmamap_t map, 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); 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, static void _bus_dmamap_count_phys(bus_dma_tag_t dmat, bus_dmamap_t map,
vm_paddr_t buf, bus_size_t buflen, int flags); vm_paddr_t buf, bus_size_t buflen, int flags);
static MALLOC_DEFINE(M_BUSDMA, "busdma", "busdma metadata"); static MALLOC_DEFINE(M_BUSDMA, "busdma", "busdma metadata");
#define dmat_alignment(dmat) ((dmat)->common.alignment) #define dmat_alignment(dmat) ((dmat)->common.alignment)
#define dmat_domain(dmat) ((dmat)->common.domain)
#define dmat_flags(dmat) ((dmat)->common.flags) #define dmat_flags(dmat) ((dmat)->common.flags)
#define dmat_lowaddr(dmat) ((dmat)->common.lowaddr) #define dmat_lowaddr(dmat) ((dmat)->common.lowaddr)
#define dmat_lockfunc(dmat) ((dmat)->common.lockfunc) #define dmat_lockfunc(dmat) ((dmat)->common.lockfunc)
#define dmat_lockfuncarg(dmat) ((dmat)->common.lockfuncarg) #define dmat_lockfuncarg(dmat) ((dmat)->common.lockfuncarg)
#include "../../kern/subr_busdma_bounce.c" #include "../../kern/subr_busdma_bounce.c"
static int
bounce_bus_dma_zone_setup(bus_dma_tag_t dmat)
{
struct bounce_zone *bz;
bus_size_t maxsize;
int error;
/* /*
* Round size up to a full page, and add one more page because
* there can always be one more boundary crossing than the
* number of pages in a transfer.
*/
maxsize = roundup2(dmat->common.maxsize, PAGE_SIZE) + PAGE_SIZE;
/* Must bounce */
if ((error = alloc_bounce_zone(dmat)) != 0)
return (error);
bz = dmat->bounce_zone;
if (ptoa(bz->total_bpages) < maxsize) {
int pages;
pages = atop(maxsize) + 1 - bz->total_bpages;
/* Add pages to our bounce pool */
if (alloc_bounce_pages(dmat, pages) < pages)
return (ENOMEM);
}
/* Performed initial allocation */
dmat->bounce_flags |= BF_MIN_ALLOC_COMP;
return (error);
}
/*
* Return true if the DMA should bounce because the start or end does not fall * Return true if the DMA should bounce because the start or end does not fall
* on a cacheline boundary (which would require a partial cacheline flush). * on a cacheline boundary (which would require a partial cacheline flush).
* COHERENT memory doesn't trigger cacheline flushes. Memory allocated by * COHERENT memory doesn't trigger cacheline flushes. Memory allocated by
* bus_dmamem_alloc() is always aligned to cacheline boundaries, and there's a * bus_dmamem_alloc() is always aligned to cacheline boundaries, and there's a
* strict rule that such memory cannot be accessed by the CPU while DMA is in * 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 safe to do * progress (or by multiple DMA engines at once), so that it's always safe to do
* full cacheline flushes even if that affects memory outside the range of a * 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 * given DMA operation that doesn't involve the full allocated buffer. If we're
▲ Show 20 Lines • Show All 116 Lines • ▼ Show 20 Lines if ((newtag->bounce_flags & BF_COHERENT) != 0) {
newtag->alloc_size = roundup2(newtag->common.maxsize, newtag->alloc_size = roundup2(newtag->common.maxsize,
dcache_line_size); dcache_line_size);
} }
if (newtag->common.lowaddr < ptoa((vm_paddr_t)Maxmem) || if (newtag->common.lowaddr < ptoa((vm_paddr_t)Maxmem) ||
newtag->common.alignment > 1) newtag->common.alignment > 1)
newtag->bounce_flags |= BF_COULD_BOUNCE; newtag->bounce_flags |= BF_COULD_BOUNCE;
if ((flags & BUS_DMA_ALLOCNOW) != 0) { if ((flags & BUS_DMA_ALLOCNOW) != 0)
struct bounce_zone *bz; error = bounce_bus_dma_zone_setup(newtag);
/* else
* Round size up to a full page, and add one more page because
* there can always be one more boundary crossing than the
* number of pages in a transfer.
*/
maxsize = roundup2(maxsize, PAGE_SIZE) + PAGE_SIZE;
/* Must bounce */
if ((error = alloc_bounce_zone(newtag)) != 0) {
free(newtag, M_DEVBUF);
return (error);
}
bz = newtag->bounce_zone;
if (ptoa(bz->total_bpages) < maxsize) {
int pages;
pages = atop(maxsize) + 1 - bz->total_bpages;
/* Add pages to our bounce pool */
if (alloc_bounce_pages(newtag, pages) < pages)
error = ENOMEM;
}
/* Performed initial allocation */
newtag->bounce_flags |= BF_MIN_ALLOC_COMP;
} else
error = 0; error = 0;
if (error != 0) if (error != 0)
free(newtag, M_DEVBUF); free(newtag, M_DEVBUF);
else else
*dmat = newtag; *dmat = newtag;
CTR4(KTR_BUSDMA, "%s returned tag %p tag flags 0x%x error %d", CTR4(KTR_BUSDMA, "%s returned tag %p tag flags 0x%x error %d",
__func__, newtag, (newtag != NULL ? newtag->common.flags : 0), __func__, newtag, (newtag != NULL ? newtag->common.flags : 0),
Show All 38 Lines while (dmat != NULL) {
dmat = NULL; dmat = NULL;
} }
} }
out: out:
CTR3(KTR_BUSDMA, "%s tag %p error %d", __func__, dmat_copy, error); CTR3(KTR_BUSDMA, "%s tag %p error %d", __func__, dmat_copy, error);
return (error); return (error);
} }
/*
* Update the domain for the tag. We may need to reallocate the zone and
* bounce pages.
*/
static int
bounce_bus_dma_tag_set_domain(bus_dma_tag_t dmat)
{
KASSERT(dmat->map_count == 0,
("bounce_bus_dma_tag_set_domain: Domain set after use.\n"));
if ((dmat->bounce_flags & BF_COULD_BOUNCE) == 0 ||
dmat->bounce_zone == NULL)
return (0);
dmat->bounce_flags &= ~BF_MIN_ALLOC_COMP;
return (bounce_bus_dma_zone_setup(dmat));
}
static bool static bool
bounce_bus_dma_id_mapped(bus_dma_tag_t dmat, vm_paddr_t buf, bus_size_t buflen) bounce_bus_dma_id_mapped(bus_dma_tag_t dmat, vm_paddr_t buf, bus_size_t buflen)
{ {
if (!might_bounce(dmat, NULL, buf, buflen)) if (!might_bounce(dmat, NULL, buf, buflen))
return (true); return (true);
return (!_bus_dmamap_pagesneeded(dmat, NULL, buf, buflen, NULL)); return (!_bus_dmamap_pagesneeded(dmat, NULL, buf, buflen, NULL));
} }
static bus_dmamap_t static bus_dmamap_t
alloc_dmamap(bus_dma_tag_t dmat, int flags) alloc_dmamap(bus_dma_tag_t dmat, int flags)
{ {
u_long mapsize; u_long mapsize;
bus_dmamap_t map; bus_dmamap_t map;
mapsize = sizeof(*map); mapsize = sizeof(*map);
mapsize += sizeof(struct sync_list) * dmat->common.nsegments; mapsize += sizeof(struct sync_list) * dmat->common.nsegments;
map = malloc(mapsize, M_DEVBUF, flags | M_ZERO); map = malloc_domainset(mapsize, M_DEVBUF,
DOMAINSET_PREF(dmat->common.domain), flags | M_ZERO);
if (map == NULL) if (map == NULL)
return (NULL); return (NULL);
/* Initialize the new map */ /* Initialize the new map */
STAILQ_INIT(&map->bpages); STAILQ_INIT(&map->bpages);
return (map); return (map);
} }
/* /*
* Allocate a handle for mapping from kva/uva/physical * Allocate a handle for mapping from kva/uva/physical
* address space into bus device space. * address space into bus device space.
*/ */
static int static int
bounce_bus_dmamap_create(bus_dma_tag_t dmat, int flags, bus_dmamap_t *mapp) bounce_bus_dmamap_create(bus_dma_tag_t dmat, int flags, bus_dmamap_t *mapp)
{ {
struct bounce_zone *bz; struct bounce_zone *bz;
int error, maxpages, pages; int error, maxpages, pages;
error = 0; error = 0;
if (dmat->segments == NULL) { if (dmat->segments == NULL) {
dmat->segments = (bus_dma_segment_t *)malloc( dmat->segments = mallocarray_domainset(dmat->common.nsegments,
sizeof(bus_dma_segment_t) * dmat->common.nsegments, sizeof(bus_dma_segment_t), M_DEVBUF,
M_DEVBUF, M_NOWAIT); DOMAINSET_PREF(dmat->common.domain), M_NOWAIT);
markjUnsubmitted
Not Done
Inline Actions
if (dmat->segments == NULL) {
- dmat->segments = (bus_dma_segment_t *)mallocarray_domainset(
+ dmat->segments = mallocarray_domainset(
dmat->common.nsegments, sizeof(bus_dma_segment_t),
markj:
if (dmat->segments == NULL) { if (dmat->segments == NULL) {
CTR3(KTR_BUSDMA, "%s: tag %p error %d", CTR3(KTR_BUSDMA, "%s: tag %p error %d",
__func__, dmat, ENOMEM); __func__, dmat, ENOMEM);
return (ENOMEM); return (ENOMEM);
} }
} }
*mapp = alloc_dmamap(dmat, M_NOWAIT); *mapp = alloc_dmamap(dmat, M_NOWAIT);
▲ Show 20 Lines • Show All 92 Lines • ▼ Show 20 Linesbounce_bus_dmamem_alloc(bus_dma_tag_t dmat, void** vaddr, int flags,
int mflags; int mflags;
if (flags & BUS_DMA_NOWAIT) if (flags & BUS_DMA_NOWAIT)
mflags = M_NOWAIT; mflags = M_NOWAIT;
else else
mflags = M_WAITOK; mflags = M_WAITOK;
if (dmat->segments == NULL) { if (dmat->segments == NULL) {
dmat->segments = (bus_dma_segment_t *)malloc( dmat->segments = mallocarray_domainset(dmat->common.nsegments,
sizeof(bus_dma_segment_t) * dmat->common.nsegments, sizeof(bus_dma_segment_t), M_DEVBUF,
M_DEVBUF, mflags); DOMAINSET_PREF(dmat->common.domain), mflags);
markjUnsubmitted
Not Done
Inline Actions
if (dmat->segments == NULL) {
- dmat->segments = (bus_dma_segment_t *)mallocarray_domainset(
+ dmat->segments = mallocarray_domainset(
dmat->common.nsegments, sizeof(bus_dma_segment_t),
markj:
if (dmat->segments == NULL) { if (dmat->segments == NULL) {
CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d", CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d",
__func__, dmat, dmat->common.flags, ENOMEM); __func__, dmat, dmat->common.flags, ENOMEM);
return (ENOMEM); return (ENOMEM);
} }
} }
if (flags & BUS_DMA_ZERO) if (flags & BUS_DMA_ZERO)
mflags |= M_ZERO; mflags |= M_ZERO;
Show All 27 Linesbounce_bus_dmamem_alloc(bus_dma_tag_t dmat, void** vaddr, int flags,
if (attr == VM_MEMATTR_UNCACHEABLE || if (attr == VM_MEMATTR_UNCACHEABLE ||
(dmat->bounce_flags & BF_COHERENT) != 0) (dmat->bounce_flags & BF_COHERENT) != 0)
(*mapp)->flags |= DMAMAP_COHERENT; (*mapp)->flags |= DMAMAP_COHERENT;
(*mapp)->flags |= DMAMAP_FROM_DMAMEM; (*mapp)->flags |= DMAMAP_FROM_DMAMEM;
/* /*
* Allocate the buffer from the malloc(9) allocator if... * Allocate the buffer from the malloc(9) allocator if...
* - It's small enough to fit into a single power of two sized bucket. * - It's small enough to fit into a single page.
* - The alignment is less than or equal to the maximum size * - Its alignment requirement is also smaller than the page size.
* - The low address requirement is fulfilled. * - The low address requirement is fulfilled.
* - Default cache attributes are requested (WB).
* else allocate non-contiguous pages if... * else allocate non-contiguous pages if...
* - The page count that could get allocated doesn't exceed * - The page count that could get allocated doesn't exceed
* nsegments also when the maximum segment size is less * nsegments also when the maximum segment size is less
* than PAGE_SIZE. * than PAGE_SIZE.
* - The alignment constraint isn't larger than a page boundary. * - The alignment constraint isn't larger than a page boundary.
* - There are no boundary-crossing constraints. * - There are no boundary-crossing constraints.
* else allocate a block of contiguous pages because one or more of the * else allocate a block of contiguous pages because one or more of the
* constraints is something that only the contig allocator can fulfill. * constraints is something that only the contig allocator can fulfill.
* *
* NOTE: The (dmat->common.alignment <= dmat->maxsize) check * NOTE: The (dmat->common.alignment <= dmat->maxsize) check
* below is just a quick hack. The exact alignment guarantees * below is just a quick hack. The exact alignment guarantees
* of malloc(9) need to be nailed down, and the code below * of malloc(9) need to be nailed down, and the code below
* should be rewritten to take that into account. * should be rewritten to take that into account.
* *
* In the meantime warn the user if malloc gets it wrong. * In the meantime warn the user if malloc gets it wrong.
*/ */
if ((dmat->alloc_size <= PAGE_SIZE) && if (dmat->alloc_size <= PAGE_SIZE &&
(dmat->alloc_alignment <= dmat->alloc_size) && dmat->alloc_alignment <= PAGE_SIZE &&
markjUnsubmitted
Not Done
Inline Actions

Why did this change?

markj: Why did this change?
andrewAuthorUnsubmitted
Done
Inline Actions

Because I switched to malloc_domainset_aligned so we could explicitly set the alignment.

I can sync the above comment with x86 as needed.

andrew: Because I switched to `malloc_domainset_aligned` so we could explicitly set the alignment. I…
dmat->common.lowaddr >= ptoa((vm_paddr_t)Maxmem) && dmat->common.lowaddr >= ptoa((vm_paddr_t)Maxmem) &&
attr == VM_MEMATTR_DEFAULT) { attr == VM_MEMATTR_DEFAULT) {
*vaddr = malloc(dmat->alloc_size, M_DEVBUF, mflags); *vaddr = malloc_domainset_aligned(dmat->alloc_size,
dmat->alloc_alignment, M_DEVBUF,
DOMAINSET_PREF(dmat->common.domain), mflags);
} else if (dmat->common.nsegments >= } else if (dmat->common.nsegments >=
howmany(dmat->alloc_size, MIN(dmat->common.maxsegsz, PAGE_SIZE)) && howmany(dmat->alloc_size, MIN(dmat->common.maxsegsz, PAGE_SIZE)) &&
dmat->alloc_alignment <= PAGE_SIZE && dmat->alloc_alignment <= PAGE_SIZE &&
(dmat->common.boundary % PAGE_SIZE) == 0) { (dmat->common.boundary % PAGE_SIZE) == 0) {
/* Page-based multi-segment allocations allowed */ /* Page-based multi-segment allocations allowed */
*vaddr = kmem_alloc_attr(dmat->alloc_size, mflags, *vaddr = kmem_alloc_attr_domainset(
0ul, dmat->common.lowaddr, attr); DOMAINSET_PREF(dmat->common.domain), dmat->alloc_size,
mflags, 0ul, dmat->common.lowaddr, attr);
dmat->bounce_flags |= BF_KMEM_ALLOC; dmat->bounce_flags |= BF_KMEM_ALLOC;
} else { } else {
*vaddr = kmem_alloc_contig(dmat->alloc_size, mflags, *vaddr = kmem_alloc_contig_domainset(
0ul, dmat->common.lowaddr, dmat->alloc_alignment != 0 ? DOMAINSET_PREF(dmat->common.domain), dmat->alloc_size,
dmat->alloc_alignment : 1ul, dmat->common.boundary, attr); mflags, 0ul, dmat->common.lowaddr,
dmat->alloc_alignment != 0 ? dmat->alloc_alignment : 1ul,
dmat->common.boundary, attr);
dmat->bounce_flags |= BF_KMEM_ALLOC; dmat->bounce_flags |= BF_KMEM_ALLOC;
} }
if (*vaddr == NULL) { if (*vaddr == NULL) {
CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d", CTR4(KTR_BUSDMA, "%s: tag %p tag flags 0x%x error %d",
__func__, dmat, dmat->common.flags, ENOMEM); __func__, dmat, dmat->common.flags, ENOMEM);
free(*mapp, M_DEVBUF); free(*mapp, M_DEVBUF);
return (ENOMEM); return (ENOMEM);
} else if (!vm_addr_align_ok(vtophys(*vaddr), dmat->alloc_alignment)) { } else if (!vm_addr_align_ok(vtophys(*vaddr), dmat->alloc_alignment)) {
▲ Show 20 Lines • Show All 559 Lines • ▼ Show 20 Lines if ((op & (BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE)) != 0) {
*/ */
dsb(sy); dsb(sy);
} }
} }
struct bus_dma_impl bus_dma_bounce_impl = { struct bus_dma_impl bus_dma_bounce_impl = {
.tag_create = bounce_bus_dma_tag_create, .tag_create = bounce_bus_dma_tag_create,
.tag_destroy = bounce_bus_dma_tag_destroy, .tag_destroy = bounce_bus_dma_tag_destroy,
.tag_set_domain = bounce_bus_dma_tag_set_domain,
.id_mapped = bounce_bus_dma_id_mapped, .id_mapped = bounce_bus_dma_id_mapped,
.map_create = bounce_bus_dmamap_create, .map_create = bounce_bus_dmamap_create,
.map_destroy = bounce_bus_dmamap_destroy, .map_destroy = bounce_bus_dmamap_destroy,
.mem_alloc = bounce_bus_dmamem_alloc, .mem_alloc = bounce_bus_dmamem_alloc,
.mem_free = bounce_bus_dmamem_free, .mem_free = bounce_bus_dmamem_free,
.load_phys = bounce_bus_dmamap_load_phys, .load_phys = bounce_bus_dmamap_load_phys,
.load_buffer = bounce_bus_dmamap_load_buffer, .load_buffer = bounce_bus_dmamap_load_buffer,
.load_ma = bus_dmamap_load_ma_triv, .load_ma = bus_dmamap_load_ma_triv,
.map_waitok = bounce_bus_dmamap_waitok, .map_waitok = bounce_bus_dmamap_waitok,
.map_complete = bounce_bus_dmamap_complete, .map_complete = bounce_bus_dmamap_complete,
.map_unload = bounce_bus_dmamap_unload, .map_unload = bounce_bus_dmamap_unload,
.map_sync = bounce_bus_dmamap_sync .map_sync = bounce_bus_dmamap_sync
}; };