Changeset View
Standalone View
sys/opencrypto/criov.c
Show First 20 Lines • Show All 233 Lines • ▼ Show 20 Lines | while (len > 0) { | ||||
processed += count; | processed += count; | ||||
off = 0; | off = 0; | ||||
pages++; | pages++; | ||||
} | } | ||||
return processed; | return processed; | ||||
} | } | ||||
#endif /* CRYPTO_MAY_HAVE_VMPAGE */ | #endif /* CRYPTO_MAY_HAVE_VMPAGE */ | ||||
/* | |||||
* Given a starting page in an m_epg, determine the length of the | |||||
* current physically contiguous segment. | |||||
*/ | |||||
static __inline size_t | |||||
m_epg_pages_extent(struct mbuf *m, int idx, u_int pglen) | |||||
markj: The indentation of the continuing line is off, ditto below. | |||||
{ | |||||
size_t len; | |||||
u_int i; | |||||
len = pglen; | |||||
for (i = idx + 1; i < m->m_epg_npgs; i++) { | |||||
if (m->m_epg_pa[i - 1] + PAGE_SIZE != m->m_epg_pa[i]) | |||||
break; | |||||
len += m_epg_pagelen(m, i, 0); | |||||
} | |||||
return (len); | |||||
Done Inline ActionsWould be nice to have a local var pagelen = m_epg_pagelen(m, i, pgoff), ditto below. markj: Would be nice to have a local var `pagelen = m_epg_pagelen(m, i, pgoff)`, ditto below. | |||||
} | |||||
static __inline void * | |||||
Done Inline ActionsI would KASSERT(offset < m_epg_trllen). markj: I would KASSERT(offset < m_epg_trllen). | |||||
m_epg_segbase(struct mbuf *m, size_t offset) | |||||
{ | |||||
u_int i, pglen, pgoff; | |||||
offset += mtod(m, vm_offset_t); | |||||
if (offset < m->m_epg_hdrlen) | |||||
return (m->m_epg_hdr + offset); | |||||
offset -= m->m_epg_hdrlen; | |||||
pgoff = m->m_epg_1st_off; | |||||
for (i = 0; i < m->m_epg_npgs; i++) { | |||||
pglen = m_epg_pagelen(m, i, pgoff); | |||||
if (offset < pglen) | |||||
return ((void *)PHYS_TO_DMAP(m->m_epg_pa[i] + pgoff + | |||||
offset)); | |||||
offset -= pglen; | |||||
pgoff = 0; | |||||
} | |||||
KASSERT(offset <= m->m_epg_trllen, ("%s: offset beyond trailer", | |||||
__func__)); | |||||
return (m->m_epg_trail + offset); | |||||
} | |||||
static __inline size_t | |||||
m_epg_seglen(struct mbuf *m, size_t offset) | |||||
{ | |||||
u_int i, pglen, pgoff; | |||||
offset += mtod(m, vm_offset_t); | |||||
if (offset < m->m_epg_hdrlen) | |||||
return (m->m_epg_hdrlen - offset); | |||||
offset -= m->m_epg_hdrlen; | |||||
pgoff = m->m_epg_1st_off; | |||||
for (i = 0; i < m->m_epg_npgs; i++) { | |||||
pglen = m_epg_pagelen(m, i, pgoff); | |||||
if (offset < pglen) | |||||
return (m_epg_pages_extent(m, i, pglen) - offset); | |||||
offset -= pglen; | |||||
pgoff = 0; | |||||
} | |||||
KASSERT(offset <= m->m_epg_trllen, ("%s: offset beyond trailer", | |||||
__func__)); | |||||
return (m->m_epg_trllen - offset); | |||||
} | |||||
Done Inline ActionsWith commit 49f6925ca34 there is a good chance that the pages are physically contiguous, it may be profitable to check for that here. markj: With commit 49f6925ca34 there is a good chance that the pages are physically contiguous, it may… | |||||
static __inline void * | |||||
m_epg_contiguous_subsegment(struct mbuf *m, size_t skip, size_t len) | |||||
{ | |||||
u_int i, pglen, pgoff; | |||||
skip += mtod(m, vm_offset_t); | |||||
if (skip < m->m_epg_hdrlen) { | |||||
Done Inline ActionsThe existence of this check implies that skip + len might be an offset past the end of the mbuf, but we appear to assume that skip alone is always within the bounds of the mbuf. It would be nice to have some assertions or comments documenting exactly what the assumptions are. markj: The existence of this check implies that skip + len might be an offset past the end of the mbuf… | |||||
Done Inline ActionsI was probably coding this a bit defensively. Looking at the caller, we check skip and len against m_len first and only invoke this if skip is < m_len. We probably should assert that. jhb: I was probably coding this a bit defensively. Looking at the caller, we check skip and len… | |||||
if (len > m->m_epg_hdrlen - skip) | |||||
return (NULL); | |||||
return (m->m_epg_hdr + skip); | |||||
} | |||||
skip -= m->m_epg_hdrlen; | |||||
pgoff = m->m_epg_1st_off; | |||||
for (i = 0; i < m->m_epg_npgs; i++) { | |||||
pglen = m_epg_pagelen(m, i, pgoff); | |||||
if (skip < pglen) { | |||||
if (len > m_epg_pages_extent(m, i, pglen) - skip) | |||||
return (NULL); | |||||
return ((void *)PHYS_TO_DMAP(m->m_epg_pa[i] + pgoff + | |||||
skip)); | |||||
} | |||||
skip -= pglen; | |||||
pgoff = 0; | |||||
} | |||||
KASSERT(skip <= m->m_epg_trllen && len <= m->m_epg_trllen - skip, | |||||
("%s: segment beyond trailer", __func__)); | |||||
return (m->m_epg_trail + skip); | |||||
} | |||||
void | void | ||||
crypto_cursor_init(struct crypto_buffer_cursor *cc, | crypto_cursor_init(struct crypto_buffer_cursor *cc, | ||||
const struct crypto_buffer *cb) | const struct crypto_buffer *cb) | ||||
{ | { | ||||
memset(cc, 0, sizeof(*cc)); | memset(cc, 0, sizeof(*cc)); | ||||
cc->cc_type = cb->cb_type; | cc->cc_type = cb->cb_type; | ||||
switch (cc->cc_type) { | switch (cc->cc_type) { | ||||
case CRYPTO_BUF_CONTIG: | case CRYPTO_BUF_CONTIG: | ||||
▲ Show 20 Lines • Show All 82 Lines • ▼ Show 20 Lines | |||||
#ifdef INVARIANTS | #ifdef INVARIANTS | ||||
panic("%s: invalid buffer type %d", __func__, cc->cc_type); | panic("%s: invalid buffer type %d", __func__, cc->cc_type); | ||||
#endif | #endif | ||||
break; | break; | ||||
} | } | ||||
} | } | ||||
void * | void * | ||||
crypto_cursor_segbase(struct crypto_buffer_cursor *cc) | crypto_cursor_segbase(struct crypto_buffer_cursor *cc) | ||||
Not Done Inline ActionsJust a general comment, I think it would be nicer and more efficient if we had a function which returned both the base address and length of the next segment. Many consumers call _segbase and _seglen back-to-back. It seems especially awkward with extpg buffers since we end up looping over the page array twice. markj: Just a general comment, I think it would be nicer and more efficient if we had a function which… | |||||
Done Inline ActionsYes, that was not as apparent for the previous types, but the double loop is a bit much, and in practice it is true that callers tend to fetch the pair. jhb: Yes, that was not as apparent for the previous types, but the double loop is a bit much, and in… | |||||
{ | { | ||||
switch (cc->cc_type) { | switch (cc->cc_type) { | ||||
case CRYPTO_BUF_CONTIG: | case CRYPTO_BUF_CONTIG: | ||||
return (cc->cc_buf); | return (cc->cc_buf); | ||||
case CRYPTO_BUF_MBUF: | case CRYPTO_BUF_MBUF: | ||||
if (cc->cc_mbuf == NULL) | if (cc->cc_mbuf == NULL) | ||||
return (NULL); | return (NULL); | ||||
KASSERT((cc->cc_mbuf->m_flags & M_EXTPG) == 0, | if (cc->cc_mbuf->m_flags & M_EXTPG) | ||||
("%s: not supported for unmapped mbufs", __func__)); | return (m_epg_segbase(cc->cc_mbuf, cc->cc_offset)); | ||||
return (mtod(cc->cc_mbuf, char *) + cc->cc_offset); | return (mtod(cc->cc_mbuf, char *) + cc->cc_offset); | ||||
case CRYPTO_BUF_VMPAGE: | case CRYPTO_BUF_VMPAGE: | ||||
return ((char *)PHYS_TO_DMAP(VM_PAGE_TO_PHYS( | return ((char *)PHYS_TO_DMAP(VM_PAGE_TO_PHYS( | ||||
*cc->cc_vmpage)) + cc->cc_offset); | *cc->cc_vmpage)) + cc->cc_offset); | ||||
case CRYPTO_BUF_UIO: | case CRYPTO_BUF_UIO: | ||||
return ((char *)cc->cc_iov->iov_base + cc->cc_offset); | return ((char *)cc->cc_iov->iov_base + cc->cc_offset); | ||||
default: | default: | ||||
#ifdef INVARIANTS | #ifdef INVARIANTS | ||||
Show All 9 Lines | crypto_cursor_seglen(struct crypto_buffer_cursor *cc) | ||||
switch (cc->cc_type) { | switch (cc->cc_type) { | ||||
case CRYPTO_BUF_CONTIG: | case CRYPTO_BUF_CONTIG: | ||||
return (cc->cc_buf_len); | return (cc->cc_buf_len); | ||||
case CRYPTO_BUF_VMPAGE: | case CRYPTO_BUF_VMPAGE: | ||||
return (PAGE_SIZE - cc->cc_offset); | return (PAGE_SIZE - cc->cc_offset); | ||||
case CRYPTO_BUF_MBUF: | case CRYPTO_BUF_MBUF: | ||||
if (cc->cc_mbuf == NULL) | if (cc->cc_mbuf == NULL) | ||||
return (0); | return (0); | ||||
if (cc->cc_mbuf->m_flags & M_EXTPG) | |||||
return (m_epg_seglen(cc->cc_mbuf, cc->cc_offset)); | |||||
return (cc->cc_mbuf->m_len - cc->cc_offset); | return (cc->cc_mbuf->m_len - cc->cc_offset); | ||||
case CRYPTO_BUF_UIO: | case CRYPTO_BUF_UIO: | ||||
return (cc->cc_iov->iov_len - cc->cc_offset); | return (cc->cc_iov->iov_len - cc->cc_offset); | ||||
default: | default: | ||||
#ifdef INVARIANTS | #ifdef INVARIANTS | ||||
panic("%s: invalid buffer type %d", __func__, cc->cc_type); | panic("%s: invalid buffer type %d", __func__, cc->cc_type); | ||||
#endif | #endif | ||||
return (0); | return (0); | ||||
Show All 13 Lines | crypto_cursor_copyback(struct crypto_buffer_cursor *cc, int size, | ||||
case CRYPTO_BUF_CONTIG: | case CRYPTO_BUF_CONTIG: | ||||
MPASS(cc->cc_buf_len >= size); | MPASS(cc->cc_buf_len >= size); | ||||
memcpy(cc->cc_buf, src, size); | memcpy(cc->cc_buf, src, size); | ||||
cc->cc_buf += size; | cc->cc_buf += size; | ||||
cc->cc_buf_len -= size; | cc->cc_buf_len -= size; | ||||
break; | break; | ||||
case CRYPTO_BUF_MBUF: | case CRYPTO_BUF_MBUF: | ||||
for (;;) { | for (;;) { | ||||
KASSERT((cc->cc_mbuf->m_flags & M_EXTPG) == 0, | /* | ||||
("%s: not supported for unmapped mbufs", __func__)); | * This uses m_copyback() for individual | ||||
dst = mtod(cc->cc_mbuf, char *) + cc->cc_offset; | * mbufs so that cc_mbuf and cc_offset are | ||||
* updated. | |||||
*/ | |||||
remain = cc->cc_mbuf->m_len - cc->cc_offset; | remain = cc->cc_mbuf->m_len - cc->cc_offset; | ||||
todo = MIN(remain, size); | todo = MIN(remain, size); | ||||
memcpy(dst, src, todo); | m_copyback(cc->cc_mbuf, cc->cc_offset, todo, src); | ||||
src += todo; | src += todo; | ||||
if (todo < remain) { | if (todo < remain) { | ||||
cc->cc_offset += todo; | cc->cc_offset += todo; | ||||
break; | break; | ||||
} | } | ||||
size -= todo; | size -= todo; | ||||
cc->cc_mbuf = cc->cc_mbuf->m_next; | cc->cc_mbuf = cc->cc_mbuf->m_next; | ||||
cc->cc_offset = 0; | cc->cc_offset = 0; | ||||
▲ Show 20 Lines • Show All 59 Lines • ▼ Show 20 Lines | crypto_cursor_copydata(struct crypto_buffer_cursor *cc, int size, void *vdst) | ||||
case CRYPTO_BUF_CONTIG: | case CRYPTO_BUF_CONTIG: | ||||
MPASS(cc->cc_buf_len >= size); | MPASS(cc->cc_buf_len >= size); | ||||
memcpy(dst, cc->cc_buf, size); | memcpy(dst, cc->cc_buf, size); | ||||
cc->cc_buf += size; | cc->cc_buf += size; | ||||
cc->cc_buf_len -= size; | cc->cc_buf_len -= size; | ||||
break; | break; | ||||
case CRYPTO_BUF_MBUF: | case CRYPTO_BUF_MBUF: | ||||
for (;;) { | for (;;) { | ||||
KASSERT((cc->cc_mbuf->m_flags & M_EXTPG) == 0, | /* | ||||
("%s: not supported for unmapped mbufs", __func__)); | * This uses m_copydata() for individual | ||||
src = mtod(cc->cc_mbuf, const char *) + cc->cc_offset; | * mbufs so that cc_mbuf and cc_offset are | ||||
* updated. | |||||
*/ | |||||
remain = cc->cc_mbuf->m_len - cc->cc_offset; | remain = cc->cc_mbuf->m_len - cc->cc_offset; | ||||
todo = MIN(remain, size); | todo = MIN(remain, size); | ||||
memcpy(dst, src, todo); | m_copydata(cc->cc_mbuf, cc->cc_offset, todo, dst); | ||||
dst += todo; | dst += todo; | ||||
if (todo < remain) { | if (todo < remain) { | ||||
cc->cc_offset += todo; | cc->cc_offset += todo; | ||||
break; | break; | ||||
} | } | ||||
size -= todo; | size -= todo; | ||||
cc->cc_mbuf = cc->cc_mbuf->m_next; | cc->cc_mbuf = cc->cc_mbuf->m_next; | ||||
cc->cc_offset = 0; | cc->cc_offset = 0; | ||||
▲ Show 20 Lines • Show All 211 Lines • ▼ Show 20 Lines | m_contiguous_subsegment(struct mbuf *m, size_t skip, size_t len) | ||||
if (m == NULL) | if (m == NULL) | ||||
return (NULL); | return (NULL); | ||||
MPASS(rel_off >= 0); | MPASS(rel_off >= 0); | ||||
skip = rel_off; | skip = rel_off; | ||||
if (skip + len > m->m_len) | if (skip + len > m->m_len) | ||||
return (NULL); | return (NULL); | ||||
if (m->m_flags & M_EXTPG) | |||||
return (m_epg_contiguous_subsegment(m, skip, len)); | |||||
return (mtod(m, char*) + skip); | return (mtod(m, char*) + skip); | ||||
} | } | ||||
Not Done Inline ActionsI don't really understand the ifdefs. On platforms without a direct map, PHYS_TO_DMAP() will panic. We don't have special ifdef guards around M_EXTPG handling elsewhere, so I'm not sure why it's required here. markj: I don't really understand the ifdefs. On platforms without a direct map, PHYS_TO_DMAP() will… | |||||
Done Inline ActionsThe m_epg_contiguous_subsegment() function is conditional on the #ifdef. I could perhaps always define the function instead and have it's body be #ifdef'ed? jhb: The m_epg_contiguous_subsegment() function is conditional on the #ifdef. I could perhaps… | |||||
Not Done Inline ActionsI see. It's more that in generic code we handle M_EXTPG mbufs unconditionally, implicitly assuming that they won't arise on platforms without a direct map. I'd maybe move m_epg_contiguous_subsegment() out of the ifdef as well. I guess that has the downside of compiling dead code on platforms which don't have a direct map, so I don't have a strong preference. It's ok as-is. markj: I see. It's more that in generic code we handle M_EXTPG mbufs unconditionally, implicitly… | |||||
Done Inline ActionsI've removed them and also removed some explicit KASSERT()'s for HAS_DMAP counting on PHYS_TO_DMAP() to panic instead. jhb: I've removed them and also removed some explicit KASSERT()'s for HAS_DMAP counting on… | |||||
static inline void * | static inline void * | ||||
cuio_contiguous_segment(struct uio *uio, size_t skip, size_t len) | cuio_contiguous_segment(struct uio *uio, size_t skip, size_t len) | ||||
{ | { | ||||
int rel_off, idx; | int rel_off, idx; | ||||
MPASS(skip <= INT_MAX); | MPASS(skip <= INT_MAX); | ||||
idx = cuio_getptr(uio, (int)skip, &rel_off); | idx = cuio_getptr(uio, (int)skip, &rel_off); | ||||
if (idx < 0) | if (idx < 0) | ||||
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines |
The indentation of the continuing line is off, ditto below.