On Tue, Oct 18, 2022 at 02:56:23PM -0700, Nathan Huckleberry wrote: > -static void internal_polyval_update(const struct polyval_tfm_ctx *keys, > +static inline struct polyval_tfm_ctx *polyval_tfm_ctx(const void *raw_ctx) > +{ > + unsigned long addr = (unsigned long)raw_ctx; > + unsigned long align = POLYVAL_ALIGN; > + > + if (align <= crypto_tfm_ctx_alignment()) > + align = 1; > + return (struct polyval_tfm_ctx *)ALIGN(addr, align); > +} This could just use PTR_ALIGN. Also, checking for POLYVAL_ALIGN <= crypto_tfm_ctx_alignment() isn't necessary. > + > +static void internal_polyval_update(const void *raw_keys, > const u8 *in, size_t nblocks, u8 *accumulator) > { > + const struct polyval_tfm_ctx *keys = polyval_tfm_ctx(raw_keys); This is being passed a struct polyval_tfm_ctx. There's no need to cast it back to a void pointer and align it again redundantly. > if (likely(crypto_simd_usable())) { > kernel_fpu_begin(); > clmul_polyval_update(keys, in, nblocks, accumulator); > @@ -102,7 +117,8 @@ static int polyval_x86_update(struct shash_desc *desc, > const u8 *src, unsigned int srclen) > { > struct polyval_desc_ctx *dctx = shash_desc_ctx(desc); > - const struct polyval_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm); > + const struct polyval_tfm_ctx *tctx = > + polyval_tfm_ctx(crypto_shash_ctx(desc->tfm)); > u8 *pos; > unsigned int nblocks; > unsigned int n; It would make more sense to have the polyval_tfm_ctx() function take in the struct crypto_shash. How about using the following: static inline struct polyval_tfm_ctx *polyval_tfm_ctx(struct crypto_shash *tfm) { return PTR_ALIGN(crypto_shash_ctx(tfm), POLYVAL_ALIGN); } - Eric